Fixed issues with previous CL.

BUG=88782
TEST=None

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

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

3 files changed, 53 insertions, 40 deletions

diff --git a/net/base/origin_bound_cert_service.cc b/net/base/origin_bound_cert_service.cc
index 853d3ef..0d706bc 100644
--- a/net/base/origin_bound_cert_service.cc
+++ b/net/base/origin_bound_cert_service.cc
@@ -7,59 +7,75 @@
 #include <limits>
 
 #include "base/logging.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/rand_util.h"
 #include "crypto/rsa_private_key.h"
+#include "googleurl/src/gurl.h"
+#include "net/base/origin_bound_cert_store.h"
 #include "net/base/x509_certificate.h"
 
 namespace net {
 
+namespace {
+
+const int kKeySizeInBits = 1024;
+const int kValidityPeriodInDays = 365;
+
+}  // namespace
+
 bool OriginBoundCertService::GetOriginBoundCert(const GURL& url,
                                                 std::string* private_key_result,
                                                 std::string* cert_result) {
   // Check if origin bound cert already exists for this origin.
   if (origin_bound_cert_store_->HasOriginBoundCert(url)) {
-    origin_bound_cert_store_->GetOriginBoundCert(url,
-                                                 private_key_result,
-                                                 cert_result);
-    return true;
+    return origin_bound_cert_store_->GetOriginBoundCert(url,
+                                                        private_key_result,
+                                                        cert_result);
   }
 
   // No origin bound cert exists, we have to create one.
-  std::string origin = GetCertOriginFromURL(url);
+  std::string origin = url.GetOrigin().spec();
   std::string subject = "CN=origin-bound certificate for " + origin;
-  X509Certificate* x509_cert;
-  crypto::RSAPrivateKey* key = crypto::RSAPrivateKey::Create(1024);
-  if ((x509_cert = X509Certificate::CreateSelfSigned(
-      key,
+  scoped_ptr<crypto::RSAPrivateKey> key(
+      crypto::RSAPrivateKey::Create(kKeySizeInBits));
+  if (!key.get()) {
+    LOG(WARNING) << "Unable to create key pair for client";
+    return false;
+  }
+  scoped_refptr<X509Certificate> x509_cert = X509Certificate::CreateSelfSigned(
+      key.get(),
       subject,
       base::RandInt(0, std::numeric_limits<int>::max()),
-      base::TimeDelta::FromDays(365))) == NULL) {
+      base::TimeDelta::FromDays(kValidityPeriodInDays));
+  if (!x509_cert) {
     LOG(WARNING) << "Unable to create x509 cert for client";
     return false;
   }
 
-  std::vector<uint8> key_vec;
-  if (!key->ExportPrivateKey(&key_vec)) {
-    LOG(WARNING) << "Unable to create x509 cert for client";
+  std::vector<uint8> private_key_info;
+  if (!key->ExportPrivateKey(&private_key_info)) {
+    LOG(WARNING) << "Unable to export private key";
     return false;
   }
-  std::string key_output(key_vec.begin(), key_vec.end());
+  // TODO(rkn): Perhaps ExportPrivateKey should be changed to output a
+  // std::string* to prevent this copying.
+  std::string key_out(private_key_info.begin(), private_key_info.end());
 
-  std::string cert_output;
-  if (!x509_cert->GetDEREncoded(&cert_output)) {
-    LOG(WARNING) << "Unable to create x509 cert for client";
+  std::string der_cert;
+  if (!x509_cert->GetDEREncoded(&der_cert)) {
+    LOG(WARNING) << "Unable to get DER-enconded cert";
     return false;
   }
 
-  origin_bound_cert_store_->SetOriginBoundCert(url, key_output, cert_output);
-  *private_key_result = key_output;
-  *cert_result = cert_output;
+  if (!origin_bound_cert_store_->SetOriginBoundCert(url, key_out, der_cert)) {
+    LOG(WARNING) << "Unable to set origin bound certificate";
+    return false;
+  }
 
+  private_key_result->swap(key_out);
+  cert_result->swap(der_cert);
   return true;
 }
 
-std::string OriginBoundCertService::GetCertOriginFromURL(const GURL& url) {
-  return url.GetOrigin().spec();
-}
-
 }  // namespace net
diff --git a/net/base/origin_bound_cert_service.h b/net/base/origin_bound_cert_service.h
index 4502247..c1d65b9 100644
--- a/net/base/origin_bound_cert_service.h
+++ b/net/base/origin_bound_cert_service.h
@@ -8,30 +8,28 @@
 
 #include <string>
 
-#include "base/basictypes.h"
-#include "net/base/origin_bound_cert_store.h"
-#include "googleurl/src/gurl.h" // TODO(rkn): This feels wrong.
+class GURL;
 
 namespace net {
 
+class OriginBoundCertStore;
+
 // A class for creating and fetching origin bound certs.
 class OriginBoundCertService {
  public:
-
-  OriginBoundCertService(OriginBoundCertStore* origin_bound_cert_store)
+  explicit OriginBoundCertService(OriginBoundCertStore* origin_bound_cert_store)
       : origin_bound_cert_store_(origin_bound_cert_store) {}
 
   // TODO(rkn): Specify certificate type (RSA or DSA).
   // TODO(rkn): Key generation can be time consuming, so this should have an
   // asynchronous interface.
-  // This function will fetch the origin bound cert for the specified origin
-  // if one exists and it will create one otherwise.
+  // Fetches the origin bound cert for the specified origin if one exists
+  // and creates one otherwise. On success, |private_key_result| stores a
+  // PrivateKeyInfo struct, and |cert_result| stores a DER-encoded certificate.
   bool GetOriginBoundCert(const GURL& url,
                           std::string* private_key_result,
                           std::string* cert_result);
 
-  static std::string GetCertOriginFromURL(const GURL& url);
-
  private:
   OriginBoundCertStore* origin_bound_cert_store_;
 };
diff --git a/net/base/origin_bound_cert_store.h b/net/base/origin_bound_cert_store.h
index 01be9e3..8529fbd 100644
--- a/net/base/origin_bound_cert_store.h
+++ b/net/base/origin_bound_cert_store.h
@@ -8,28 +8,27 @@
 
 #include <string>
 
-#include "base/basictypes.h"
-
 class GURL;
 
 namespace net {
 
-// An interface for storing and retrieving origin bound certs.
+// An interface for storing and retrieving origin bound certs. Origin bound
+// certificates are specified in
+// http://balfanz.github.com/tls-obc-spec/draft-balfanz-tls-obc-00.html.
 
 class OriginBoundCertStore {
  public:
-
   virtual bool HasOriginBoundCert(const GURL& url) = 0;
 
   // TODO(rkn): Specify certificate type (RSA or DSA).
   // TODO(rkn): Key generation can be time consuming, so this should have an
   // asynchronous interface.
-  // The output is stored in |private_key| and |cert|.
-  virtual void GetOriginBoundCert(const GURL& url,
+  // The output is stored in |private_key_result| and |cert_result|.
+  virtual bool GetOriginBoundCert(const GURL& url,
                                   std::string* private_key_result,
                                   std::string* cert_result) = 0;
 
-  virtual void SetOriginBoundCert(const GURL& url,
+  virtual bool SetOriginBoundCert(const GURL& url,
                                   const std::string& private_key,
                                   const std::string& cert) = 0;
 };