Use scoped objects to simplify crypto routines. Also do lots of other simplification.

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/3888002

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

6 files changed, 110 insertions, 166 deletions

diff --git a/base/crypto/capi_util.cc b/base/crypto/capi_util.cc
index 0499492..cf47a50 100644
--- a/base/crypto/capi_util.cc
+++ b/base/crypto/capi_util.cc
@@ -38,8 +38,8 @@ class CAPIUtilSingleton {
 namespace base {
 
 BOOL CryptAcquireContextLocked(HCRYPTPROV* prov,
-                               const TCHAR* container,
-                               const TCHAR* provider,
+                               LPCWSTR container,
+                               LPCWSTR provider,
                                DWORD prov_type,
                                DWORD flags)
 {
diff --git a/base/crypto/capi_util.h b/base/crypto/capi_util.h
index 8f89828..df7f749 100644
--- a/base/crypto/capi_util.h
+++ b/base/crypto/capi_util.h
@@ -22,8 +22,8 @@ namespace base {
 // CRYPT_NEWKEYSET or CRYPT_DELETEKEYSET is specified in the dwFlags
 // parameter."
 BOOL CryptAcquireContextLocked(HCRYPTPROV* prov,
-                               const TCHAR* container,
-                               const TCHAR* provider,
+                               LPCWSTR container,
+                               LPCWSTR provider,
                                DWORD prov_type,
                                DWORD flags);
 
diff --git a/base/crypto/rsa_private_key_win.cc b/base/crypto/rsa_private_key_win.cc
index 5dd8cca..6c8a34b 100644
--- a/base/crypto/rsa_private_key_win.cc
+++ b/base/crypto/rsa_private_key_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -96,15 +96,15 @@ RSAPrivateKey* RSAPrivateKey::CreateFromPrivateKeyInfo(
   dest += pki.exponent2()->size();
   memcpy(dest, &pki.coefficient()->front(), pki.coefficient()->size());
   dest += pki.coefficient()->size();
-  memcpy(dest, &pki.private_exponent()->front(), pki.private_exponent()->size());
+  memcpy(dest, &pki.private_exponent()->front(),
+         pki.private_exponent()->size());
   dest += pki.private_exponent()->size();
 
   READ_ASSERT(dest == blob.get() + blob_size);
-  if (!CryptImportKey(
-      result->provider_, reinterpret_cast<uint8*>(public_key_struc),
-      blob_size, NULL, CRYPT_EXPORTABLE, result->key_.receive())) {
+  if (!CryptImportKey(result->provider_,
+                      reinterpret_cast<uint8*>(public_key_struc), blob_size, 0,
+                      CRYPT_EXPORTABLE, result->key_.receive()))
     return NULL;
-  }
 
   return result.release();
 }
@@ -135,14 +135,13 @@ bool RSAPrivateKey::InitProvider() {
 bool RSAPrivateKey::ExportPrivateKey(std::vector<uint8>* output) {
   // Export the key
   DWORD blob_length = 0;
-  if (!CryptExportKey(key_, NULL, PRIVATEKEYBLOB, 0, NULL, &blob_length)) {
+  if (!CryptExportKey(key_, 0, PRIVATEKEYBLOB, 0, NULL, &blob_length)) {
     NOTREACHED();
     return false;
   }
 
   scoped_array<uint8> blob(new uint8[blob_length]);
-  if (!CryptExportKey(key_, NULL, PRIVATEKEYBLOB, 0, blob.get(),
-                      &blob_length)) {
+  if (!CryptExportKey(key_, 0, PRIVATEKEYBLOB, 0, blob.get(), &blob_length)) {
     NOTREACHED();
     return false;
   }
diff --git a/base/crypto/symmetric_key_win.cc b/base/crypto/symmetric_key_win.cc
index 76be8ad..87d715a 100644
--- a/base/crypto/symmetric_key_win.cc
+++ b/base/crypto/symmetric_key_win.cc
@@ -48,8 +48,10 @@ ALG_ID GetAESAlgIDForKeySize(size_t key_size_in_bits) {
 // CALG_HMAC.
 // If successful, returns true and stores the imported key in |*key|.
 // TODO(wtc): use this function in hmac_win.cc.
-bool ImportRawKey(HCRYPTPROV provider, ALG_ID alg, const void* key_data,
-                  DWORD key_size, ScopedHCRYPTKEY* key) {
+bool ImportRawKey(HCRYPTPROV provider,
+                  ALG_ID alg,
+                  const void* key_data, DWORD key_size,
+                  ScopedHCRYPTKEY* key) {
   DCHECK_GT(key_size, 0);
 
   DWORD actual_size = sizeof(PlaintextBlobHeader) + key_size;
@@ -77,8 +79,8 @@ bool ImportRawKey(HCRYPTPROV provider, ALG_ID alg, const void* key_data,
     flags |= CRYPT_IPSEC_HMAC_KEY;
   }
 
-  BOOL ok = CryptImportKey(provider, actual_key, actual_size, NULL,
-                           flags, &unsafe_key);
+  BOOL ok =
+      CryptImportKey(provider, actual_key, actual_size, 0, flags, &unsafe_key);
 
   // Clean up the temporary copy of key, regardless of whether it was imported
   // sucessfully or not.
@@ -94,7 +96,8 @@ bool ImportRawKey(HCRYPTPROV provider, ALG_ID alg, const void* key_data,
 // Attempts to generate a random AES key of |key_size_in_bits|. Returns true
 // if generation is successful, storing the generated key in |*key| and the
 // key provider (CSP) in |*provider|.
-bool GenerateAESKey(size_t key_size_in_bits, ScopedHCRYPTPROV* provider,
+bool GenerateAESKey(size_t key_size_in_bits,
+                    ScopedHCRYPTPROV* provider,
                     ScopedHCRYPTKEY* key) {
   DCHECK(provider);
   DCHECK(key);
@@ -161,8 +164,10 @@ bool CheckHMACKeySize(size_t key_size_in_bits, ALG_ID alg) {
 // |key_size_in_bits| must be >= 1/2 the hash size of |alg| for security.
 // Returns true if generation is successful, storing the generated key in
 // |*key| and the key provider (CSP) in |*provider|.
-bool GenerateHMACKey(size_t key_size_in_bits, ALG_ID alg,
-                     ScopedHCRYPTPROV* provider, ScopedHCRYPTKEY* key,
+bool GenerateHMACKey(size_t key_size_in_bits,
+                     ALG_ID alg,
+                     ScopedHCRYPTPROV* provider,
+                     ScopedHCRYPTKEY* key,
                      scoped_array<BYTE>* raw_key) {
   DCHECK(provider);
   DCHECK(key);
@@ -202,7 +207,9 @@ bool GenerateHMACKey(size_t key_size_in_bits, ALG_ID alg,
 // and |key|. The inner hash function will be |hash_alg|. If successful,
 // returns true and stores the hash in |*hash|.
 // TODO(wtc): use this function in hmac_win.cc.
-bool CreateHMACHash(HCRYPTPROV provider, HCRYPTKEY key, ALG_ID hash_alg,
+bool CreateHMACHash(HCRYPTPROV provider,
+                    HCRYPTKEY key,
+                    ALG_ID hash_alg,
                     ScopedHCRYPTHASH* hash) {
   ScopedHCRYPTHASH safe_hash;
   BOOL ok = CryptCreateHash(provider, CALG_HMAC, key, 0, safe_hash.receive());
@@ -228,9 +235,12 @@ bool CreateHMACHash(HCRYPTPROV provider, HCRYPTKEY key, ALG_ID hash_alg,
 // |output_buf| must have enough space to accomodate the output of the PRF
 // specified by |hash|.
 // Returns true if the block was successfully computed.
-bool ComputePBKDF2Block(HCRYPTHASH hash, DWORD hash_size,
-                        const std::string& salt, size_t iterations,
-                        uint32 block_index, BYTE* output_buf) {
+bool ComputePBKDF2Block(HCRYPTHASH hash,
+                        DWORD hash_size,
+                        const std::string& salt,
+                        size_t iterations,
+                        uint32 block_index,
+                        BYTE* output_buf) {
   // From RFC 2898:
   // 3. <snip> The function F is defined as the exclusive-or sum of the first
   //    c iterates of the underlying pseudorandom function PRF applied to the
@@ -247,9 +257,8 @@ bool ComputePBKDF2Block(HCRYPTHASH hash, DWORD hash_size,
     return false;
 
   // Iteration U_1: Compute PRF for S.
-  ok = CryptHashData(safe_hash,
-                     reinterpret_cast<const BYTE*>(salt.data()), salt.size(),
-                     0);
+  ok = CryptHashData(safe_hash, reinterpret_cast<const BYTE*>(salt.data()),
+                     salt.size(), 0);
   if (!ok)
     return false;
 
@@ -432,10 +441,9 @@ SymmetricKey* SymmetricKey::DeriveKeyFromPassword(Algorithm algorithm,
   //    a derived key DK:
   //    DK = T_1 || T_2 || ... || T_l<0..r-1>
   for (uint32 block_index = 1; block_index <= L; ++block_index) {
-    if (!ComputePBKDF2Block(prf, hLen, salt, iterations,
-                            block_index, block_offset)) {
+    if (!ComputePBKDF2Block(prf, hLen, salt, iterations, block_index,
+                            block_offset))
         return NULL;
-    }
     block_offset += hLen;
   }
 
@@ -474,8 +482,8 @@ SymmetricKey* SymmetricKey::Import(Algorithm algorithm,
     return NULL;
 
   ScopedHCRYPTPROV provider;
-  BOOL ok = CryptAcquireContext(provider.receive(), NULL, NULL,
-                                provider_type, CRYPT_VERIFYCONTEXT);
+  BOOL ok = CryptAcquireContext(provider.receive(), NULL, NULL, provider_type,
+                                CRYPT_VERIFYCONTEXT);
   if (!ok)
     return NULL;
 
@@ -495,13 +503,13 @@ bool SymmetricKey::GetRawKey(std::string* raw_key) {
   }
 
   DWORD size = 0;
-  BOOL ok = CryptExportKey(key_, NULL, PLAINTEXTKEYBLOB, 0, NULL, &size);
+  BOOL ok = CryptExportKey(key_, 0, PLAINTEXTKEYBLOB, 0, NULL, &size);
   if (!ok)
     return false;
 
   std::vector<BYTE> result(size);
 
-  ok = CryptExportKey(key_, NULL, PLAINTEXTKEYBLOB, 0, &result[0], &size);
+  ok = CryptExportKey(key_, 0, PLAINTEXTKEYBLOB, 0, &result[0], &size);
   if (!ok)
     return false;
 
@@ -515,7 +523,8 @@ bool SymmetricKey::GetRawKey(std::string* raw_key) {
   return true;
 }
 
-SymmetricKey::SymmetricKey(HCRYPTPROV provider, HCRYPTKEY key,
+SymmetricKey::SymmetricKey(HCRYPTPROV provider,
+                           HCRYPTKEY key,
                            const void* key_data, size_t key_size_in_bytes)
     : provider_(provider), key_(key) {
   if (key_data) {
diff --git a/base/hmac_win.cc b/base/hmac_win.cc
index 03f81db..a1c8225 100644
--- a/base/hmac_win.cc
+++ b/base/hmac_win.cc
@@ -10,6 +10,7 @@
 #include <algorithm>
 #include <vector>
 
+#include "base/crypto/scoped_capi_types.h"
 #include "base/logging.h"
 #include "base/third_party/nss/blapi.h"
 #include "base/third_party/nss/sha256.h"
@@ -72,12 +73,17 @@ void ComputeHMACSHA256(const unsigned char* key, size_t key_len,
 }  // namespace
 
 struct HMACPlatformData {
-  HMACPlatformData() : provider_(0), hash_(0), hkey_(0) {}
+  ~HMACPlatformData() {
+    if (!raw_key_.empty()) {
+      SecureZeroMemory(&raw_key_[0], raw_key_.size());
+    }
 
-  // Windows Crypt API resources.
-  HCRYPTPROV provider_;
-  HCRYPTHASH hash_;
-  HCRYPTKEY hkey_;
+    // Destroy the key before releasing the provider.
+    key_.reset();
+  }
+
+  ScopedHCRYPTPROV provider_;
+  ScopedHCRYPTKEY key_;
 
   // For HMAC-SHA-256 only.
   std::vector<unsigned char> raw_key_;
@@ -89,8 +95,8 @@ HMAC::HMAC(HashAlgorithm hash_alg)
   DCHECK(hash_alg_ == SHA1 || hash_alg_ == SHA256);
 }
 
-bool HMAC::Init(const unsigned char *key, int key_length) {
-  if (plat_->provider_ || plat_->hkey_ || !plat_->raw_key_.empty()) {
+bool HMAC::Init(const unsigned char* key, int key_length) {
+  if (plat_->provider_ || plat_->key_ || !plat_->raw_key_.empty()) {
     // Init must not be called more than once on the same HMAC object.
     NOTREACHED();
     return false;
@@ -103,10 +109,9 @@ bool HMAC::Init(const unsigned char *key, int key_length) {
     return true;
   }
 
-  if (!CryptAcquireContext(&plat_->provider_, NULL, NULL,
+  if (!CryptAcquireContext(plat_->provider_.receive(), NULL, NULL,
                            PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) {
     NOTREACHED();
-    plat_->provider_ = NULL;
     return false;
   }
 
@@ -135,9 +140,8 @@ bool HMAC::Init(const unsigned char *key, int key_length) {
 
   if (!CryptImportKey(plat_->provider_, &key_blob_storage[0],
                       key_blob_storage.size(), 0, CRYPT_IPSEC_HMAC_KEY,
-                      &plat_->hkey_)) {
+                      plat_->key_.receive())) {
     NOTREACHED();
-    plat_->hkey_ = NULL;
     return false;
   }
 
@@ -148,22 +152,6 @@ bool HMAC::Init(const unsigned char *key, int key_length) {
 }
 
 HMAC::~HMAC() {
-  if (!plat_->raw_key_.empty())
-    SecureZeroMemory(&plat_->raw_key_[0], plat_->raw_key_.size());
-
-  BOOL ok;
-  if (plat_->hkey_) {
-    ok = CryptDestroyKey(plat_->hkey_);
-    DCHECK(ok);
-  }
-  if (plat_->hash_) {
-    ok = CryptDestroyHash(plat_->hash_);
-    DCHECK(ok);
-  }
-  if (plat_->provider_) {
-    ok = CryptReleaseContext(plat_->provider_, 0);
-    DCHECK(ok);
-  }
 }
 
 bool HMAC::Sign(const std::string& data,
@@ -178,7 +166,7 @@ bool HMAC::Sign(const std::string& data,
     return true;
   }
 
-  if (!plat_->provider_ || !plat_->hkey_)
+  if (!plat_->provider_ || !plat_->key_)
     return false;
 
   if (hash_alg_ != SHA1) {
@@ -186,27 +174,24 @@ bool HMAC::Sign(const std::string& data,
     return false;
   }
 
-  if (!CryptCreateHash(
-          plat_->provider_, CALG_HMAC, plat_->hkey_, 0, &plat_->hash_))
+  ScopedHCRYPTHASH hash;
+  if (!CryptCreateHash(plat_->provider_, CALG_HMAC, plat_->key_, 0,
+                       hash.receive()))
     return false;
 
   HMAC_INFO hmac_info;
   memset(&hmac_info, 0, sizeof(hmac_info));
   hmac_info.HashAlgid = CALG_SHA1;
-  if (!CryptSetHashParam(plat_->hash_, HP_HMAC_INFO,
+  if (!CryptSetHashParam(hash, HP_HMAC_INFO,
                          reinterpret_cast<BYTE*>(&hmac_info), 0))
     return false;
 
-  if (!CryptHashData(plat_->hash_,
-                     reinterpret_cast<const BYTE*>(data.data()),
+  if (!CryptHashData(hash, reinterpret_cast<const BYTE*>(data.data()),
                      static_cast<DWORD>(data.size()), 0))
     return false;
 
   DWORD sha1_size = digest_length;
-  if (!CryptGetHashParam(plat_->hash_, HP_HASHVAL, digest, &sha1_size, 0))
-    return false;
-
-  return true;
+  return !!CryptGetHashParam(hash, HP_HASHVAL, digest, &sha1_size, 0);
 }
 
 }  // namespace base
diff --git a/base/sha1_win.cc b/base/sha1_win.cc
index 0d0cf2c..853c244 100644
--- a/base/sha1_win.cc
+++ b/base/sha1_win.cc
@@ -7,108 +7,59 @@
 #include <windows.h>
 #include <wincrypt.h>
 
+#include "base/crypto/scoped_capi_types.h"
 #include "base/logging.h"
 
 namespace base {
 
-// Implementation of SHA-1 using Windows CryptoAPI.
-
-// Usage example:
-//
-// SecureHashAlgorithm sha;
-// while(there is data to hash)
-//   sha.Update(moredata, size of data);
-// sha.Final();
-// memcpy(somewhere, sha.Digest(), 20);
-//
-// to reuse the instance of sha, call sha.Init();
-
-class SecureHashAlgorithm {
- public:
-  SecureHashAlgorithm() : prov_(NULL), hash_(NULL) { Init(); }
-
-  void Init();
-  void Update(const void* data, size_t nbytes);
-  void Final();
-
-  // 20 bytes of message digest.
-  const unsigned char* Digest() const {
-    return reinterpret_cast<const unsigned char*>(result_.data());
-  }
-
- private:
-  // Cleans up prov_, hash_, and result_.
-  void Cleanup();
-
-  HCRYPTPROV prov_;
-  HCRYPTHASH hash_;
-  std::string result_;
-};
-
-void SecureHashAlgorithm::Init() {
-  Cleanup();
-
-  if (!CryptAcquireContext(&prov_, 0, 0, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) {
+std::string SHA1HashString(const std::string& str) {
+  ScopedHCRYPTPROV provider;
+  if (!CryptAcquireContext(provider.receive(), NULL, NULL, PROV_RSA_FULL,
+                           CRYPT_VERIFYCONTEXT)) {
     LOG(ERROR) << "CryptAcquireContext failed: " << GetLastError();
-    return;
-  }
-
-  // Initialize the hash.
-  if (!CryptCreateHash(prov_, CALG_SHA1, 0, 0, &hash_)) {
-    LOG(ERROR) << "CryptCreateHash failed: " << GetLastError();
-    return;
+    return std::string(SHA1_LENGTH, '\0');
   }
-}
-
-void SecureHashAlgorithm::Update(const void* data, size_t nbytes) {
-  BOOL ok = CryptHashData(hash_, reinterpret_cast<CONST BYTE*>(data),
-                          static_cast<DWORD>(nbytes), 0);
-  CHECK(ok) << "CryptHashData failed: " << GetLastError();
-}
-
-void SecureHashAlgorithm::Final() {
-  DWORD hash_len = 0;
-  DWORD buffer_size = sizeof(hash_len);
-  if (!CryptGetHashParam(hash_, HP_HASHSIZE,
-                         reinterpret_cast<unsigned char*>(&hash_len),
-                         &buffer_size, 0)) {
-    LOG(ERROR) << "CryptGetHashParam(HP_HASHSIZE) failed: " << GetLastError();
-    result_.assign(SHA1_LENGTH, '\0');
-    return;
-  }
-
-  // Get the hash data.
-  if (!CryptGetHashParam(hash_, HP_HASHVAL,
-                         reinterpret_cast<BYTE*>(WriteInto(&result_,
-                                                           hash_len + 1)),
-                         &hash_len, 0)) {
-    LOG(ERROR) << "CryptGetHashParam(HP_HASHVAL) failed: " << GetLastError();
-    result_.assign(SHA1_LENGTH, '\0');
-    return;
-  }
-}
 
-void SecureHashAlgorithm::Cleanup() {
-  BOOL ok;
-  if (hash_) {
-    ok = CryptDestroyHash(hash_);
-    DCHECK(ok);
-    hash_ = NULL;
-  }
-  if (prov_) {
-    ok = CryptReleaseContext(prov_, 0);
-    DCHECK(ok);
-    prov_ = NULL;
+  {
+    ScopedHCRYPTHASH hash;
+    if (!CryptCreateHash(provider, CALG_SHA1, 0, 0, hash.receive())) {
+      LOG(ERROR) << "CryptCreateHash failed: " << GetLastError();
+      return std::string(SHA1_LENGTH, '\0');
+    }
+
+    if (!CryptHashData(hash, reinterpret_cast<CONST BYTE*>(str.data()),
+                       static_cast<DWORD>(str.length()), 0)) {
+      LOG(ERROR) << "CryptHashData failed: " << GetLastError();
+      return std::string(SHA1_LENGTH, '\0');
+    }
+
+    DWORD hash_len = 0;
+    DWORD buffer_size = sizeof hash_len;
+    if (!CryptGetHashParam(hash, HP_HASHSIZE,
+                           reinterpret_cast<unsigned char*>(&hash_len),
+                           &buffer_size, 0)) {
+      LOG(ERROR) << "CryptGetHashParam(HP_HASHSIZE) failed: " << GetLastError();
+      return std::string(SHA1_LENGTH, '\0');
+    }
+
+    std::string result;
+    if (!CryptGetHashParam(hash, HP_HASHVAL,
+        // We need the + 1 here not because the call will write a trailing \0,
+        // but so that result.length() is correctly set to |hash_len|.
+        reinterpret_cast<BYTE*>(WriteInto(&result, hash_len + 1)), &hash_len,
+        0))) {
+      LOG(ERROR) << "CryptGetHashParam(HP_HASHVAL) failed: " << GetLastError();
+      return std::string(SHA1_LENGTH, '\0');
+    }
+
+    if (hash_len != SHA1_LENGTH) {
+      LOG(ERROR) << "Returned hash value is wrong length: " << hash_len
+                 << " should be " << SHA1_LENGTH;
+      return std::string(SHA1_LENGTH, '\0');
+    }
+
+    return result;
   }
-  result_.clear();
-}
-
-std::string SHA1HashString(const std::string& str) {
-  SecureHashAlgorithm sha;
-  sha.Update(str.c_str(), str.length());
-  sha.Final();
-  std::string out(reinterpret_cast<const char*>(sha.Digest()), SHA1_LENGTH);
-  return out;
 }
 
 }  // namespace base