Fix SpdySession::WriteCredentialFrame ECPrivateKey creation args.

It was passing the DER certificate instead of a SubjectPublicKeyInfo.

Also adds ECSignatureCreator::SetFactoryForTesting method to allow
easier testing of code that uses ECSignatureCreator.

BUG=none
TEST=SpdyHttpStreamTest.SendCredentialsEC


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

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

7 files changed, 95 insertions, 36 deletions

diff --git a/crypto/crypto.gyp b/crypto/crypto.gyp
index 1d216f6..92d6a0e 100644
--- a/crypto/crypto.gyp
+++ b/crypto/crypto.gyp
@@ -140,7 +140,9 @@
         'ec_private_key.h',
         'ec_private_key_nss.cc',
         'ec_private_key_openssl.cc',
+        'ec_signature_creator.cc',
         'ec_signature_creator.h',
+        'ec_signature_creator_impl.h',
         'ec_signature_creator_nss.cc',
         'ec_signature_creator_openssl.cc',
         'encryptor.cc',
diff --git a/crypto/ec_signature_creator.cc b/crypto/ec_signature_creator.cc
new file mode 100644
index 0000000..cbaa820
--- /dev/null
+++ b/crypto/ec_signature_creator.cc
@@ -0,0 +1,30 @@
+// Copyright (c) 2012 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 "crypto/ec_signature_creator.h"
+
+#include "crypto/ec_signature_creator_impl.h"
+
+namespace crypto {
+
+namespace {
+
+ECSignatureCreatorFactory* g_factory_ = NULL;
+
+}  // namespace
+
+// static
+ECSignatureCreator* ECSignatureCreator::Create(ECPrivateKey* key) {
+  if (g_factory_)
+    return g_factory_->Create(key);
+  return new ECSignatureCreatorImpl(key);
+}
+
+// static
+void ECSignatureCreator::SetFactoryForTesting(
+    ECSignatureCreatorFactory* factory) {
+  g_factory_ = factory;
+}
+
+}  // namespace crypto
diff --git a/crypto/ec_signature_creator.h b/crypto/ec_signature_creator.h
index 8858eb5..610b9f0 100644
--- a/crypto/ec_signature_creator.h
+++ b/crypto/ec_signature_creator.h
@@ -14,13 +14,21 @@
 namespace crypto {
 
 class ECPrivateKey;
+class ECSignatureCreator;
+
+class CRYPTO_EXPORT ECSignatureCreatorFactory {
+ public:
+  virtual ~ECSignatureCreatorFactory() {}
+
+  virtual ECSignatureCreator* Create(ECPrivateKey* key) = 0;
+};
 
 // Signs data using a bare private key (as opposed to a full certificate).
 // We need this class because SignatureCreator is hardcoded to use
 // RSAPrivateKey.
 class CRYPTO_EXPORT ECSignatureCreator {
  public:
-  ~ECSignatureCreator();
+  virtual ~ECSignatureCreator() {}
 
   // Create an instance. The caller must ensure that the provided PrivateKey
   // instance outlives the created ECSignatureCreator.
@@ -28,23 +36,21 @@ class CRYPTO_EXPORT ECSignatureCreator {
   // pass in the hash algorithm identifier.
   static ECSignatureCreator* Create(ECPrivateKey* key);
 
+  // Set a factory to make the Create function return non-standard
+  // ECSignatureCreator objects.  Because the ECDSA algorithm involves
+  // randomness, this is useful for higher-level tests that want to have
+  // deterministic mocked output to compare.
+  static void SetFactoryForTesting(ECSignatureCreatorFactory* factory);
+
   // Signs |data_len| bytes from |data| and writes the results into
   // |signature| as a DER encoded ECDSA-Sig-Value from RFC 3279.
   //
   //  ECDSA-Sig-Value ::= SEQUENCE {
   //    r     INTEGER,
   //    s     INTEGER }
-  bool Sign(const uint8* data,
-            int data_len,
-            std::vector<uint8>* signature);
-
- private:
-  // Private constructor. Use the Create() method instead.
-  explicit ECSignatureCreator(ECPrivateKey* key);
-
-  ECPrivateKey* key_;
-
-  DISALLOW_COPY_AND_ASSIGN(ECSignatureCreator);
+  virtual bool Sign(const uint8* data,
+                    int data_len,
+                    std::vector<uint8>* signature) = 0;
 };
 
 }  // namespace crypto
diff --git a/crypto/ec_signature_creator_impl.h b/crypto/ec_signature_creator_impl.h
new file mode 100644
index 0000000..6f17ef6
--- /dev/null
+++ b/crypto/ec_signature_creator_impl.h
@@ -0,0 +1,31 @@
+// Copyright (c) 2012 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.
+
+#ifndef CRYPTO_EC_SIGNATURE_CREATOR_IMPL_H_
+#define CRYPTO_EC_SIGNATURE_CREATOR_IMPL_H_
+#pragma once
+
+#include "base/compiler_specific.h"
+#include "crypto/ec_signature_creator.h"
+
+namespace crypto {
+
+class ECSignatureCreatorImpl : public ECSignatureCreator {
+ public:
+  explicit ECSignatureCreatorImpl(ECPrivateKey* key);
+  virtual ~ECSignatureCreatorImpl();
+
+  virtual bool Sign(const uint8* data,
+                    int data_len,
+                    std::vector<uint8>* signature) OVERRIDE;
+
+ private:
+  ECPrivateKey* key_;
+
+  DISALLOW_COPY_AND_ASSIGN(ECSignatureCreatorImpl);
+};
+
+}  // namespace crypto
+
+#endif  // CRYPTO_EC_SIGNATURE_CREATOR_IMPL_H_
diff --git a/crypto/ec_signature_creator_nss.cc b/crypto/ec_signature_creator_nss.cc
index 933f1cc..388870f 100644
--- a/crypto/ec_signature_creator_nss.cc
+++ b/crypto/ec_signature_creator_nss.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "crypto/ec_signature_creator.h"
+#include "crypto/ec_signature_creator_impl.h"
 
 #include <cryptohi.h>
 #include <pk11pub.h>
@@ -50,21 +50,16 @@ SECStatus SignData(SECItem* result,
 
 }  // namespace
 
-// static
-ECSignatureCreator* ECSignatureCreator::Create(ECPrivateKey* key) {
-  return new ECSignatureCreator(key);
-}
-
-ECSignatureCreator::ECSignatureCreator(ECPrivateKey* key)
+ECSignatureCreatorImpl::ECSignatureCreatorImpl(ECPrivateKey* key)
     : key_(key) {
   EnsureNSSInit();
 }
 
-ECSignatureCreator::~ECSignatureCreator() { }
+ECSignatureCreatorImpl::~ECSignatureCreatorImpl() {}
 
-bool ECSignatureCreator::Sign(const uint8* data,
-                              int data_len,
-                              std::vector<uint8>* signature) {
+bool ECSignatureCreatorImpl::Sign(const uint8* data,
+                                  int data_len,
+                                  std::vector<uint8>* signature) {
   // Data to be signed
   SECItem secret;
   secret.type = siBuffer;
diff --git a/crypto/ec_signature_creator_openssl.cc b/crypto/ec_signature_creator_openssl.cc
index 9fa8ddc..ae443c4 100644
--- a/crypto/ec_signature_creator_openssl.cc
+++ b/crypto/ec_signature_creator_openssl.cc
@@ -2,28 +2,22 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "crypto/ec_signature_creator.h"
+#include "crypto/ec_signature_creator_impl.h"
 
 #include "base/logging.h"
 
 namespace crypto {
 
-// static
-ECSignatureCreator* ECSignatureCreator::Create(ECPrivateKey* key) {
-  NOTIMPLEMENTED();
-  return NULL;
-}
-
-ECSignatureCreator::ECSignatureCreator(ECPrivateKey* key)
+ECSignatureCreatorImpl::ECSignatureCreatorImpl(ECPrivateKey* key)
     : key_(key) {
   NOTIMPLEMENTED();
 }
 
-ECSignatureCreator::~ECSignatureCreator() { }
+ECSignatureCreatorImpl::~ECSignatureCreatorImpl() {}
 
-bool ECSignatureCreator::Sign(const uint8* data,
-                              int data_len,
-                              std::vector<uint8>* signature) {
+bool ECSignatureCreatorImpl::Sign(const uint8* data,
+                                  int data_len,
+                                  std::vector<uint8>* signature) {
   NOTIMPLEMENTED();
   return false;
 }
diff --git a/crypto/ec_signature_creator_unittest.cc b/crypto/ec_signature_creator_unittest.cc
index def8a63..407b278 100644
--- a/crypto/ec_signature_creator_unittest.cc
+++ b/crypto/ec_signature_creator_unittest.cc
@@ -19,7 +19,8 @@
 TEST(ECSignatureCreatorTest, OpenSSLStub) {
   scoped_ptr<crypto::ECSignatureCreator> signer(
       crypto::ECSignatureCreator::Create(NULL));
-  ASSERT_FALSE(signer.get());
+  ASSERT_TRUE(signer.get());
+  EXPECT_FALSE(signer->Sign(NULL, 0, NULL));
 }
 #else
 TEST(ECSignatureCreatorTest, BasicTest) {