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 Delegate {
+   public:
+    Delegate() {}
+    virtual ~Delegate() {}
+
+    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;

[Ryan Sleevi, 2015/06/23 15:27:29]

All of this should be documented as to what the expectations are of
implementers.

I'm totally cool with just saying something to the effect of

// These methods behave the same as those of of the same name on SSLPrivateKey,
but must
// be callable on any thread.

(Or something, if you have a preference)

[davidben, 2015/06/24 21:43:12]

Done. Though I slightly tweaked the "callback on any thread" rule to mention
it's always called on the TaskRunner.

+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Delegate);
+  };
+
+  ThreadedSSLPrivateKey(scoped_ptr<Delegate> delegate,
+                        const scoped_refptr<base::TaskRunner>& task_runner);

[Ryan Sleevi, 2015/06/23 15:27:29]

Is the API contract TaskRunner or SequencedTaskRunner? Does it make a difference
if multiple invocations were executing in parallel?

(My gut is that we either want SequencedTaskRunner or SingleThreadTaskRunner as
the API contract for these; really, the strong requirement comes from the
concrete implementations, but since under the Liskov Substitution Principle you
really don't want concrete impls requiring stronger pre-conditions than their
base class, we should virally propagate this to the base class as the strong API
guarantee)

[davidben, 2015/06/24 21:43:13]

Hrm. I think I would say that ThreadedSSLPrivateKey should take a
base::TaskRunner because it internally doesn't care. Instead it promises that
SignDigest will be called on the TaskRunner.

Then ssl_platform_key.h takes a SingleThreadTaskRunner parameter, passes it to
ThreadedSSLPrivateKey along with the delegate that is potentially buggy. I
definitely agree that ssl_platform_key.h should take a SingleThreadTaskRunner
because that's where the single-threaded constraint is coming from, so done on
that.

I don't think this new one breaks Liskov? (If you disagree, I'm happy to bleed
the SingleThreadTaskRunner further though.)

[davidben, 2015/06/24 21:44:41]

(Er, s/SingleThread/Sequenced/. The WorkerPool vends out Sequenced. It's going
to be SingleThreaded anyway but it didn't seem worth caring too much.)

+  ~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:
+  class Core;
+
+  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_