Generalize crypto::SignatureCreator to allow choice of hash function, so as to support SHA256 (not just SHA1).

BUG=412531

R=rsleevi@chromium.org,davidben@chromium.org

TBR=pfeldman@chromium.org

Review URL: https://codereview.chromium.org/560583002

Cr-Commit-Position: refs/heads/master@{#295747}

chrome/browser/devtools/device/usb/android_rsa.cc

@@ -262,8 +262,9 @@ std::string AndroidRSASign(crypto::RSAPrivateKey* key,
                            const std::string& body) {
   std::vector<uint8> digest(body.begin(), body.end());
   std::vector<uint8> result;
-  if (!crypto::SignatureCreator::Sign(key, vector_as_array(&digest),
-                                      digest.size(), &result)) {
+  if (!crypto::SignatureCreator::Sign(key, crypto::SignatureCreator::SHA1,
+                                      vector_as_array(&digest), digest.size(),
+                                      &result)) {
     return std::string();
   }
   return std::string(result.begin(), result.end());

chrome/browser/extensions/extension_creator.cc

@@ -211,7 +211,8 @@ bool ExtensionCreator::SignZip(const base::FilePath& zip_path,
                                crypto::RSAPrivateKey* private_key,
                                std::vector<uint8>* signature) {
   scoped_ptr<crypto::SignatureCreator> signature_creator(
-      crypto::SignatureCreator::Create(private_key));
+      crypto::SignatureCreator::Create(private_key,
+                                       crypto::SignatureCreator::SHA1));
   base::ScopedFILE zip_handle(base::OpenFile(zip_path, "rb"));
   size_t buffer_size = 1 << 16;
   scoped_ptr<uint8[]> buffer(new uint8[buffer_size]);

components/ownership/owner_settings_service.cc

@@ -32,7 +32,8 @@ std::string AssembleAndSignPolicy(scoped_ptr<em::PolicyData> policy,
 
   // Generate the signature.
   scoped_ptr<crypto::SignatureCreator> signature_creator(
-      crypto::SignatureCreator::Create(private_key));
+      crypto::SignatureCreator::Create(private_key,
+                                       crypto::SignatureCreator::SHA1));
   signature_creator->Update(
       reinterpret_cast<const uint8*>(policy_response.policy_data().c_str()),
       policy_response.policy_data().size());

components/policy/core/common/cloud/policy_builder.cc

@@ -295,7 +295,8 @@ void PolicyBuilder::SignData(const std::string& data,
                              crypto::RSAPrivateKey* key,
                              std::string* signature) {
   scoped_ptr<crypto::SignatureCreator> signature_creator(
-      crypto::SignatureCreator::Create(key));
+      crypto::SignatureCreator::Create(key,
+                                       crypto::SignatureCreator::SHA1));
   signature_creator->Update(reinterpret_cast<const uint8*>(data.c_str()),
                             data.size());
   std::vector<uint8> signature_bytes;

crypto/signature_creator.h

@@ -24,18 +24,27 @@ namespace crypto {
 class RSAPrivateKey;
 
 // Signs data using a bare private key (as opposed to a full certificate).
-// Currently can only sign data using SHA-1 with RSA encryption.
+// Currently can only sign data using SHA-1 or SHA-256 with RSA PKCS#1v1.5.
 class CRYPTO_EXPORT SignatureCreator {
  public:
+  // The set of supported hash functions. Extend as required.
+  enum HashAlgorithm {
+    SHA1,
+    SHA256,
+  };
+
   ~SignatureCreator();
 
   // Create an instance. The caller must ensure that the provided PrivateKey
-  // instance outlives the created SignatureCreator.
-  static SignatureCreator* Create(RSAPrivateKey* key);
+  // instance outlives the created SignatureCreator. Uses the HashAlgorithm
+  // specified.
+  static SignatureCreator* Create(RSAPrivateKey* key, HashAlgorithm hash_alg);
 
-  // Signs the precomputed SHA-1 digest |data| using private |key| as
+
+  // Signs the precomputed |hash_alg| digest |data| using private |key| as
   // specified in PKCS #1 v1.5.
   static bool Sign(RSAPrivateKey* key,
+                   HashAlgorithm hash_alg,
                    const uint8* data,
                    int data_len,
                    std::vector<uint8>* signature);

crypto/signature_creator_nss.cc

@@ -15,6 +15,30 @@
 
 namespace crypto {
 
+namespace {
+
+SECOidTag ToNSSSigOid(SignatureCreator::HashAlgorithm hash_alg) {
+  switch (hash_alg) {
+    case SignatureCreator::SHA1:
+      return SEC_OID_PKCS1_SHA1_WITH_RSA_ENCRYPTION;
+    case SignatureCreator::SHA256:
+      return SEC_OID_PKCS1_SHA256_WITH_RSA_ENCRYPTION;
+  }
+  return SEC_OID_UNKNOWN;
+}
+
+SECOidTag ToNSSHashOid(SignatureCreator::HashAlgorithm hash_alg) {
+  switch (hash_alg) {
+    case SignatureCreator::SHA1:
+      return SEC_OID_SHA1;
+    case SignatureCreator::SHA256:
+      return SEC_OID_SHA256;
+  }
+  return SEC_OID_UNKNOWN;
+}
+
+}  // namespace
+
 SignatureCreator::~SignatureCreator() {
   if (sign_context_) {
     SGN_DestroyContext(sign_context_, PR_TRUE);
@@ -23,12 +47,12 @@ SignatureCreator::~SignatureCreator() {
 }
 
 // static
-SignatureCreator* SignatureCreator::Create(RSAPrivateKey* key) {
+SignatureCreator* SignatureCreator::Create(RSAPrivateKey* key,
+                                           HashAlgorithm hash_alg) {
   scoped_ptr<SignatureCreator> result(new SignatureCreator);
   result->key_ = key;
 
-  result->sign_context_ = SGN_NewContext(SEC_OID_PKCS1_SHA1_WITH_RSA_ENCRYPTION,
-      key->key());
+  result->sign_context_ = SGN_NewContext(ToNSSSigOid(hash_alg), key->key());
   if (!result->sign_context_) {
     NOTREACHED();
     return NULL;
@@ -45,6 +69,7 @@ SignatureCreator* SignatureCreator::Create(RSAPrivateKey* key) {
 
 // static
 bool SignatureCreator::Sign(RSAPrivateKey* key,
+                            HashAlgorithm hash_alg,
                             const uint8* data,
                             int data_len,
                             std::vector<uint8>* signature) {
@@ -54,7 +79,7 @@ bool SignatureCreator::Sign(RSAPrivateKey* key,
   data_item.len = data_len;
 
   SECItem signature_item;
-  SECStatus rv = SGN_Digest(key->key(), SEC_OID_SHA1, &signature_item,
+  SECStatus rv = SGN_Digest(key->key(), ToNSSHashOid(hash_alg), &signature_item,
                             &data_item);
   if (rv != SECSuccess) {
     NOTREACHED();

crypto/signature_creator_openssl.cc

@@ -16,18 +16,49 @@
 
 namespace crypto {
 
+namespace {
+
+const EVP_MD* ToOpenSSLDigest(SignatureCreator::HashAlgorithm hash_alg) {
+  switch (hash_alg) {
+    case SignatureCreator::SHA1:
+      return EVP_sha1();
+    case SignatureCreator::SHA256:
+      return EVP_sha256();
+  }
+  return NULL;
+}
+
+int ToOpenSSLDigestType(SignatureCreator::HashAlgorithm hash_alg) {
+  switch (hash_alg) {
+    case SignatureCreator::SHA1:
+      return NID_sha1;
+    case SignatureCreator::SHA256:
+      return NID_sha256;
+  }
+  return NID_undef;
+}
+
+}  // namespace
+
 // static
-SignatureCreator* SignatureCreator::Create(RSAPrivateKey* key) {
+SignatureCreator* SignatureCreator::Create(RSAPrivateKey* key,
+                                           HashAlgorithm hash_alg) {
   OpenSSLErrStackTracer err_tracer(FROM_HERE);
   scoped_ptr<SignatureCreator> result(new SignatureCreator);
   result->key_ = key;
-  if (!EVP_SignInit_ex(result->sign_context_, EVP_sha1(), NULL))
+  const EVP_MD* const digest = ToOpenSSLDigest(hash_alg);
+  DCHECK(digest);
+  if (!digest) {
+    return NULL;
+  }
+  if (!EVP_SignInit_ex(result->sign_context_, digest, NULL))
     return NULL;
   return result.release();
 }
 
 // static
 bool SignatureCreator::Sign(RSAPrivateKey* key,
+                            HashAlgorithm hash_alg,
                             const uint8* data,
                             int data_len,
                             std::vector<uint8>* signature) {
@@ -37,8 +68,8 @@ bool SignatureCreator::Sign(RSAPrivateKey* key,
   signature->resize(RSA_size(rsa_key.get()));
 
   unsigned int len = 0;
-  bool success = RSA_sign(NID_sha1, data, data_len, vector_as_array(signature),
-                          &len, rsa_key.get());
+  bool success = RSA_sign(ToOpenSSLDigestType(hash_alg), data, data_len,
+                          vector_as_array(signature), &len, rsa_key.get());
   if (!success) {
     signature->clear();
     return false;

crypto/signature_creator_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/sha1.h"
 #include "crypto/rsa_private_key.h"
+#include "crypto/sha2.h"
 #include "crypto/signature_creator.h"
 #include "crypto/signature_verifier.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -19,6 +20,12 @@ const uint8 kSHA1WithRSAAlgorithmID[] = {
   0xf7, 0x0d, 0x01, 0x01, 0x05, 0x05, 0x00
 };
 
+// This is the algorithm ID for SHA-1 with RSA encryption.
+const uint8 kSHA256WithRSAAlgorithmID[] = {
+  0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86,
+  0xf7, 0x0d, 0x01, 0x01, 0x0B, 0x05, 0x00
+};
+
 }
 
 TEST(SignatureCreatorTest, BasicTest) {
@@ -34,7 +41,8 @@ TEST(SignatureCreatorTest, BasicTest) {
   ASSERT_TRUE(key.get());
 
   scoped_ptr<crypto::SignatureCreator> signer(
-      crypto::SignatureCreator::Create(key.get()));
+      crypto::SignatureCreator::Create(key.get(),
+                                       crypto::SignatureCreator::SHA1));
   ASSERT_TRUE(signer.get());
 
   std::string data("Hello, World!");
@@ -76,6 +84,7 @@ TEST(SignatureCreatorTest, SignDigestTest) {
   std::vector<uint8> signature;
   ASSERT_TRUE(crypto::SignatureCreator::Sign(
       key.get(),
+      crypto::SignatureCreator::SHA1,
       reinterpret_cast<const uint8*>(sha1.c_str()),
       sha1.size(),
       &signature));
@@ -94,3 +103,41 @@ TEST(SignatureCreatorTest, SignDigestTest) {
                         data.size());
   ASSERT_TRUE(verifier.VerifyFinal());
 }
+
+TEST(SignatureCreatorTest, SignSHA256DigestTest) {
+  // Do a verify round trip.
+  scoped_ptr<crypto::RSAPrivateKey> key_original(
+      crypto::RSAPrivateKey::Create(1024));
+  ASSERT_TRUE(key_original.get());
+
+  std::vector<uint8> key_info;
+  key_original->ExportPrivateKey(&key_info);
+  scoped_ptr<crypto::RSAPrivateKey> key(
+      crypto::RSAPrivateKey::CreateFromPrivateKeyInfo(key_info));
+  ASSERT_TRUE(key.get());
+
+  std::string data("Hello, World!");
+  std::string sha256 = crypto::SHA256HashString(data);
+  // Sign sha256 of the input data.
+  std::vector<uint8> signature;
+  ASSERT_TRUE(crypto::SignatureCreator::Sign(
+      key.get(),
+      crypto::SignatureCreator::HashAlgorithm::SHA256,
+      reinterpret_cast<const uint8*>(sha256.c_str()),
+      sha256.size(),
+      &signature));
+
+  std::vector<uint8> public_key_info;
+  ASSERT_TRUE(key_original->ExportPublicKey(&public_key_info));
+
+  // Verify the input data.
+  crypto::SignatureVerifier verifier;
+  ASSERT_TRUE(verifier.VerifyInit(
+      kSHA256WithRSAAlgorithmID, sizeof(kSHA256WithRSAAlgorithmID),
+      &signature.front(), signature.size(),
+      &public_key_info.front(), public_key_info.size()));
+
+  verifier.VerifyUpdate(reinterpret_cast<const uint8*>(data.c_str()),
+                        data.size());
+  ASSERT_TRUE(verifier.VerifyFinal());
+}

remoting/base/rsa_key_pair.cc

@@ -78,7 +78,8 @@ std::string RsaKeyPair::GetPublicKey() const {
 
 std::string RsaKeyPair::SignMessage(const std::string& message) const {
   scoped_ptr<crypto::SignatureCreator> signature_creator(
-      crypto::SignatureCreator::Create(key_.get()));
+      crypto::SignatureCreator::Create(key_.get(),
+                                       crypto::SignatureCreator::SHA1));
   signature_creator->Update(reinterpret_cast<const uint8*>(message.c_str()),
                             message.length());
   std::vector<uint8> signature_buf;