Implement TLS Channel ID support for SPDY CREDENTIAL frames

net/spdy/spdy_credential_builder.cc

Index: net/spdy/spdy_credential_builder.cc
diff --git a/net/spdy/spdy_credential_builder.cc b/net/spdy/spdy_credential_builder.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8ea0870bbbe5d030892ed49c7084361f7ba788cf
--- /dev/null
+++ b/net/spdy/spdy_credential_builder.cc
@@ -0,0 +1,79 @@
+// 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/logging.h"
+#include "base/string_piece.h"
+#include "crypto/ec_private_key.h"
+#include "crypto/ec_signature_creator.h"
+#include "crypto/signature_creator.h"
+#include "net/base/asn1_util.h"
+#include "net/base/server_bound_cert_service.h"
+#include "net/base/net_errors.h"
+#include "net/socket/ssl_client_socket.h"
+#include "net/spdy/spdy_framer.h"
+
+namespace net {
+
+// static
+int SpdyCredentialBuilder::Build(std::string tls_unique,
+                                 SSLClientCertType type,
+                                 const std::string& key,
+                                 const std::string& cert,
+                                 size_t slot,
+                                 SpdyCredential* credential) {
+  DCHECK(type == CLIENT_CERT_ECDSA_SIGN);
+  if (type != CLIENT_CERT_ECDSA_SIGN)
+    return ERR_BAD_SSL_CLIENT_AUTH_CERT;
+
+  std::string secret = SpdyCredentialBuilder::GetCredentialSecret(tls_unique);
+
+  // Extract the SubjectPublicKeyInfo from the certificate.
+  base::StringPiece spki;
+  if(!asn1::ExtractSPKIFromDERCert(cert, &spki))
+    return ERR_BAD_SSL_CLIENT_AUTH_CERT;
+
+  // Next, extract the SubjectPublicKey data, which will actually
+  // be stored in the cert field of the credential frame.
+  base::StringPiece spk;

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

nit: abbreviations are discouraged..

WTC said that that spki is very common <sigh>.... but it gets harder to read
when a second abreviation is only different by one character (the trailing i).

...how about....

spki -->  public_key_info

spk --> public_key

spk_data --> public_key_vector

or even shorter in this context....

key_info
key
key_vector

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

Done
I like this, but it's problematic because I already have "key".

+  if (!asn1::ExtractSubjectPublicKeyFromSPKI(spki, &spk))
+    return ERR_BAD_SSL_CLIENT_AUTH_CERT;
+  // Drop one byte of padding bits count from the BIT STRING
+  // (this will always be zero).  Drop one byte of X9.62 format specification
+  // (this will always be 4 to indicated an uncompressed point)
+  DCHECK_EQ(0, (int)spk[0]);

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

overly nit: period at the end of comments (here and line# 50) please.

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

(weird, your comment is off by a line again.  Is this a problem with
codereview?)

Done.

+  DCHECK_EQ(4, (int)spk[1]);

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

Consider static_cast<int> rather than C style.

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

Done.

+  spk = spk.substr(2, spk.length());
+

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

nit: should we DCHECK_GE(spk.length(), 2)? 

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

Done.

+  // Convert the strings into a vector<unit8>
+  std::vector<uint8> spki_data(spki.data(),
+                               spki.data() + spki.size());

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

nit: no need to wrap.

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

Heh, the longer names complicated this :>  I ended up using a tiny helper
method.

+  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()));
+  creator->Sign(reinterpret_cast<const unsigned char *>(secret.data()),
+                secret.length(), &proof_data);
+
+  credential->slot = slot;
+  credential->certs.push_back(spk.as_string());
+  credential->proof.assign(proof_data.begin(), proof_data.end());

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

nit: it was confusing for me to store SubjectPublicKey in certs field. Should we
consider renaming credential->certs? It is your call.

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

I hear you.  Until we change the spec, I'd prefer to leave the name of the field
alone.  But we should definitely fix this in the spec.

+  return OK;
+}
+
+// static
+std::string SpdyCredentialBuilder::GetCredentialSecret(std::string tls_unique) {
+  const char prefix[] = "SPDY CREDENTIAL ChannelID\0client -> server";
+  std::string secret(prefix, arraysize(prefix));
+  secret.append(tls_unique);
+
+  return secret;
+}
+
+}  // namespace net