Sign CertificateVerify messages on a background thread.

net/ssl/threaded_ssl_private_key.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_THREADED_SSL_PRIVATE_KEY_H_
+#define NET_SSL_THREADED_SSL_PRIVATE_KEY_H_
+
+#include <stdint.h>
+
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/strings/string_piece.h"
+#include "net/ssl/ssl_private_key.h"
+
+namespace base {
+class TaskRunner;
+}
+
+namespace net {
+
+// An SSLPrivateKey implementation which offloads key operations to a background
+// thread.
+class ThreadedSSLPrivateKey : public SSLPrivateKey {
+ public:
+  // Interface for consumers to implement to perform the actual signing
+  // operation. All methods must be callable on any thread.
+  class Core : public base::RefCountedThreadSafe<Core> {

[Ryan Sleevi, 2015/06/12 23:37:20]

The naming of Core is a little weird here. Aren't these things normally called
Delegates?

And is it necessary to expose Ref-Counting as part of the public interface? I
pretty much NACK anything new doing this unless there's a strong reason. It
seems like the threading core should be an internal implementation detail,
rather than the public API contract.

Finally, does the Core even need to be subclassable? Could it just be embodied
in platform-specific implementations? Why or why not?

[davidben, 2015/06/15 21:28:25]

Done.
Per out-of-band discussion, Core is now internal and ref-counted, hiding the
ref-counts from Delegate.
This is mostly so we don't have to repeat the threaded logic in all the
implementations, since we have two Windows ones, a Mac one. We'll have an
Android and NSS one. Maybe we'll need a CHAPS one later. I dunno.

+   public:
+    Core() {}
+
+    virtual Type GetType() = 0;
+    virtual bool SupportsHash(Hash hash) = 0;
+    virtual size_t GetMaxSignatureLength() = 0;
+    virtual Error SignDigest(Hash hash,
+                             const base::StringPiece& input,
+                             std::vector<uint8_t>* signature) = 0;
+
+   protected:
+    virtual ~Core() {}
+
+   private:
+    friend class base::RefCountedThreadSafe<Core>;
+
+    DISALLOW_COPY_AND_ASSIGN(Core);
+  };
+
+  ThreadedSSLPrivateKey(const scoped_refptr<Core>& core,
+                        const scoped_refptr<base::TaskRunner>& task_runner);
+  ~ThreadedSSLPrivateKey() override;
+
+  // SSLPrivateKey implementation.
+  Type GetType() override;
+  bool SupportsHash(Hash hash) override;
+  size_t GetMaxSignatureLength() override;
+  void SignDigest(Hash hash,
+                  const base::StringPiece& input,
+                  const SignCallback& callback) override;
+
+ private:
+  // A wrapper over SignCallback to ensure the callback isn't called if the key
+  // is destroyed.
+  void DoCallback(const SignCallback& callback,
+                  Error error,
+                  const std::vector<uint8_t>& signature);

[Ryan Sleevi, 2015/06/12 23:37:20]

There's no need to make this a private class member;

You could have the static helper in your .cc, pass it a WeakPtr to the
ThreadedSSLPrivateKey, and have the helper check that this runs.

Mostly, I'm trying to avoid exposing the implementation details in headers; this
works, but exposes how publicly.

[davidben, 2015/06/15 21:28:25]

Now it needs to be. If we split up Delegate and Core, it needs to have access to
the class's private bits.

+
+  scoped_refptr<Core> core_;
+  scoped_refptr<base::TaskRunner> task_runner_;
+  base::WeakPtrFactory<ThreadedSSLPrivateKey> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThreadedSSLPrivateKey);
+};
+
+}  // namespace net
+
+#endif  // NET_SSL_THREADED_SSL_PRIVATE_KEY_H_