Implement TLS Channel ID support for SPDY CREDENTIAL frames

net/spdy/spdy_credential_builder_unittest.cc

+// 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 "net/spdy/spdy_credential_builder.h"
+
+#include "base/threading/sequenced_worker_pool.h"
+#include "crypto/ec_signature_creator.h"
+#include "crypto/ec_private_key.h"
+#include "net/base/asn1_util.h"
+#include "net/base/default_server_bound_cert_store.h"
+#include "net/base/server_bound_cert_service.h"
+#include "net/spdy/spdy_test_util_spdy3.h"
+#include "testing/platform_test.h"
+
+using namespace net::test_spdy3;
+
+namespace net {
+
+namespace {
+
+const static size_t kSlot = 2;
+const static char kSecretPrefix[] =
+    "SPDY CREDENTIAL ChannelID\0client -> server";
+
+class MockBuilder : public SpdyCredentialBuilder {

[ramant (doing other things), 2012/08/02 00:03:05]

overly nit: do we need this class? Can we call
SpdyCredentialBuilder::GetCredentialSecret and add the tests that call
GetCredentialSecret as FRIEND_TEST_ALL_PREFIXES in SpdyCredentialBuilder?

[Ryan Hamilton, 2012/08/02 15:53:49]

As I've moved GetCredentialSecret from protected to private, and simply added a
friend class SpdyCredentialBuilderTest.

+ public:
+  std::string GetCredentialSecret() {

[ramant (doing other things), 2012/08/02 00:03:05]

nit: why GetCredentialSecret public?

[ramant (doing other things), 2012/08/02 00:54:20]

Please ignore this.

+    return SpdyCredentialBuilder::GetCredentialSecret(
+        MockClientSocket::kTlsUnique);
+  }
+};
+
+void CreateCertAndKey(std::string* cert, std::string* key) {
+  scoped_refptr<base::SequencedWorkerPool> sequenced_worker_pool =
+      new base::SequencedWorkerPool(1, "SpdyHttpStreamSpdy3Test");

[ramant (doing other things), 2012/08/02 00:03:05]

nit: consider using "CreateCertAndKey" or "SpdyCredentialBuilder", or does this
need to be  "SpdyHttpStreamSpdy3Test" for the code to work. It is your call.

[Ryan Hamilton, 2012/08/02 15:53:49]

I don't know what you mean?  The method is already called "CreateCertAndKey",
and SpdyCredentialBuilder does not know how to create certificates.  I think I'm
not understanding you.

+  scoped_ptr<ServerBoundCertService> server_bound_cert_service(

[ramant (doing other things), 2012/08/02 00:03:05]

overly nit: It would be really awesome to share code with
ServerBoundCertServiceTest

[Ryan Hamilton, 2012/08/02 15:53:49]

Done.

+      new ServerBoundCertService(new DefaultServerBoundCertStore(NULL),
+                                 sequenced_worker_pool));
+
+  TestCompletionCallback callback;
+  std::vector<uint8> requested_cert_types;
+  requested_cert_types.push_back(CLIENT_CERT_ECDSA_SIGN);
+  SSLClientCertType cert_type;
+  ServerBoundCertService::RequestHandle request_handle;
+  int rv = server_bound_cert_service->GetDomainBoundCert(
+      "https://www.google.com", requested_cert_types, &cert_type, key, cert,
+      callback.callback(), &request_handle);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_EQ(OK, callback.WaitForResult());
+  EXPECT_EQ(CLIENT_CERT_ECDSA_SIGN, cert_type);
+
+  sequenced_worker_pool->Shutdown();
+}
+
+}  // namespace
+
+class SpdyCredentialBuilderTest : public testing::Test {
+ public:
+  SpdyCredentialBuilderTest() {
+    CreateCertAndKey(&cert_, &key_);
+  }
+
+ protected:
+  int BuildWithType(SSLClientCertType type) {
+    return builder_.Build(
+        MockClientSocket::kTlsUnique, type, key_, cert_, kSlot, &credential_);
+  }
+
+  int Build() {
+    return BuildWithType(CLIENT_CERT_ECDSA_SIGN);
+  }
+
+  SpdyTestStateHelper helper_;

[jar (doing other things), 2012/08/02 02:23:04]

nit: all members should be private.

[Ryan Hamilton, 2012/08/02 15:53:49]

Normally, this is true, but the style guide specifically permits this behavior:

  "... we allow data members of a test fixture class to be protected when using
Google Test)."
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Access...

+  std::string cert_;

[ramant (doing other things), 2012/08/02 00:03:05]

nit: do we need helper_?

[Ryan Hamilton, 2012/08/02 15:53:49]

(you comment seems to be one line below the line you are actually commenting on.
 Is this a bug in codereview?)

Yes definitely.  Among other things, the helper ensures that we will have
deterministic EC signatures.  I've added a comment.

+  std::string key_;
+  MockBuilder builder_;
+  SpdyCredential credential_;
+};
+
+TEST_F(SpdyCredentialBuilderTest, GetCredentialSecret) {
+  std::string secret_str(kSecretPrefix, arraysize(kSecretPrefix));
+  secret_str.append(MockClientSocket::kTlsUnique);
+
+  EXPECT_EQ(secret_str, builder_.GetCredentialSecret());
+}
+
+TEST_F(SpdyCredentialBuilderTest, SucceedsWithECDSACert) {
+  EXPECT_EQ(OK, BuildWithType(CLIENT_CERT_ECDSA_SIGN));
+}
+
+TEST_F(SpdyCredentialBuilderTest, FailsWithRSACert) {
+#if !defined(DCHECK_ALWAYS_ON)
+  EXPECT_DEBUG_DEATH(BuildWithType(CLIENT_CERT_RSA_SIGN), "");
+#else
+  EXPECT_EQ(ERR_BAD_SSL_CLIENT_AUTH_CERT,
+            BuildWithType(CLIENT_CERT_RSA_SIGN));
+#endif
+}
+
+TEST_F(SpdyCredentialBuilderTest, SetsSlotCorrectly) {
+  ASSERT_EQ(OK, Build());
+  EXPECT_EQ(kSlot, credential_.slot);
+}
+
+TEST_F(SpdyCredentialBuilderTest, SetsCertCorrectly) {
+  ASSERT_EQ(OK, Build());
+  base::StringPiece spki;
+  ASSERT_TRUE(asn1::ExtractSPKIFromDERCert(cert_, &spki));
+  base::StringPiece spk;
+  ASSERT_TRUE(asn1::ExtractSubjectPublicKeyFromSPKI(spki, &spk));
+  EXPECT_EQ(1u, credential_.certs.size());
+  EXPECT_EQ(0, (int)spk[0]);
+  EXPECT_EQ(4, (int)spk[1]);
+  EXPECT_EQ(spk.substr(2, spk.length()).as_string(), credential_.certs[0]);
+}
+
+TEST_F(SpdyCredentialBuilderTest, SetsProofCorrectly) {
+  ASSERT_EQ(OK, Build());
+  base::StringPiece spki;
+  ASSERT_TRUE(asn1::ExtractSPKIFromDERCert(cert_, &spki));
+  std::vector<uint8> spki_data(spki.data(),
+                               spki.data() + spki.size());
+  std::vector<uint8> key_data(key_.data(),
+                              key_.data() + key_.length());
+  std::vector<uint8> proof_data;
+  scoped_ptr<crypto::ECPrivateKey> private_key(
+      crypto::ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
+          ServerBoundCertService::kEPKIPassword, key_data, spki_data));
+  scoped_ptr<crypto::ECSignatureCreator> creator(
+      crypto::ECSignatureCreator::Create(private_key.get()));
+  std::string secret = builder_.GetCredentialSecret();
+  creator->Sign(reinterpret_cast<const unsigned char *>(secret.data()),
+                secret.length(), &proof_data);
+
+  std::string proof(proof_data.begin(), proof_data.end());
+  EXPECT_EQ(proof, credential_.proof);
+}
+
+}  // namespace net