Add a ClientKeyStore to allow injection of non-platform keys for TLS client auth.

net/ssl/client_key_store.h

+// Copyright 2015 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 NET_SSL_CLIENT_KEY_STORE_H_
+#define NET_SSL_CLIENT_KEY_STORE_H_
+
+#include <map>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "base/synchronization/lock.h"
+#include "net/base/net_export.h"
+
+template <typename T>
+struct DefaultSingletonTraits;
+
+namespace net {
+
+class SSLPrivateKey;
+class X509Certificate;
+
+// TODO(rsleevi, davidben): Remove this once https://crbug.com/394131 is fixed.
+// A certificate and key store that allows several external certificate
+// providers to push certificates and keys into this store. All currently
+// provided certificates will be exposed through |FetchClientCertPrivateKey|.
+// Methods of this singleton can be called from any thread.
+class NET_EXPORT ClientKeyStore {
+ public:
+  class CertKeyProvider {
+   public:
+    // This can be called from any thread.
+    virtual ~CertKeyProvider() {}
+
+    // Obtains a handle to the certificate private key for |cert| and stores it
+    // in |private_key|.
+    // If the CertKeyProvider does not know about the |cert|, returns false. If
+    // it knows about the certificate, but is unable to return the private key,
+    // returns true and sets |*private_key| to nullptr.
+    // This can be called from any thread.
+    virtual bool GetCertificateKey(const X509Certificate& cert,
+                                   scoped_ptr<SSLPrivateKey>* private_key) = 0;
+  };
+
+  static ClientKeyStore* GetInstance();
+
+  // The |provider| will be accessed and destroyed on any thread but no
+  // concurrent method/destructor invocations will happen.
+  // |provider| will be valid until it is removed using |RemoveProvider| or the
+  // store is destroyed.
+  void AddProvider(scoped_ptr<CertKeyProvider> provider);

[davidben, 2015/08/14 22:16:48]

Given that stapling it to a Singleton is leaking it anyway, would it make more
sense to make this a raw pointer and then say that the pointer must remain valid
until RemoveProvider is called? I'm trying to understand what passing ownership
to a global means, whereas it seems the entity that calls RemoveProvider really
owns this thing.

(Is the caller uploaded yet? I don't know how you intend to call this function,
so it's possible with that context this API is better.)

[pneubeck (no reviews), 2015/08/17 14:34:19]

No the caller isn't written yet (didn't finish that part of the refactoring(s)
yet).

I actually considered only adding a provider for the new API once and never
removing it.

But, I agree that the ownership here is a bit weird and to even allow a
reasonable removal, I had to comment on the minimal lifetime of the provider.

I changed the signature to drop the scoped_ptr, as it avoids this problem.

+
+  // Removes |provider| from this store. Afterwards, |provider| is invalid.
+  void RemoveProvider(const CertKeyProvider* provider);
+
+  // Given a |certificate|'s public key, return the corresponding private
+  // key if any of the registered providers has a matching key.
+  // Returns its matching private key on success, nullptr otherwise.
+  scoped_ptr<SSLPrivateKey> FetchClientCertPrivateKey(
+      const X509Certificate& certificate);
+
+ private:
+  friend struct DefaultSingletonTraits<ClientKeyStore>;

[davidben, 2015/08/14 22:16:48]

I think LazyInstance is usually preferred over Singleton.

https://www.chromium.org/developers/coding-style/important-abstractions-and-d...

[pneubeck (no reviews), 2015/08/17 14:34:19]

I fear the numbers don't agree with that documentation:
https://code.google.com/p/chromium/codesearch#search/&q=Singleton%5C%3C%20fil...
https://code.google.com/p/chromium/codesearch#search/&q=lazyinstance%5C%3C%20...

Another difference that is documented in lazy_instance.h :
// LazyInstance is similar to Singleton, except it does not have the singleton
// property.

which would again speak for a singleton...
anyways, I don't think that it makes a significant difference.

+
+  ClientKeyStore();
+  ~ClientKeyStore();
+
+  base::Lock lock_;
+  ScopedVector<CertKeyProvider> providers_;
+
+  DISALLOW_COPY_AND_ASSIGN(ClientKeyStore);
+};
+
+}  // namespace net
+
+#endif  // NET_SSL_CLIENT_KEY_STORE_H_