Make the mapping from client id -> secret asynchronous.

In the original design, I intended the pairing registry delegate to maintain
an in-memory cache of client id -> secret, and for this to be invalidated by
a watcher when the on-disk version changes (for example, if the web-app is
used to revoke a pairing). I think an asynchronous model makes more sense,
and eliminates the need for the watcher.

In addition, this CL fixes a bug in the negotiating authenticator when dealing
with authenticators that process messages asychronously.

BUG=156182

Review URL: https://chromiumcodereview.appspot.com/16893002

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

9 files changed, 207 insertions, 119 deletions

diff --git a/remoting/protocol/negotiating_authenticator_base.cc b/remoting/protocol/negotiating_authenticator_base.cc
index 2c2b4c5..30bd8ba 100644
--- a/remoting/protocol/negotiating_authenticator_base.cc
+++ b/remoting/protocol/negotiating_authenticator_base.cc
@@ -52,6 +52,7 @@ void NegotiatingAuthenticatorBase::ProcessMessageInternal(
     // If the message was not discarded and the authenticator is waiting for it,
     // give it to the underlying authenticator to process.
     // |current_authenticator_| is owned, so Unretained() is safe here.
+    state_ = PROCESSING_MESSAGE;
     current_authenticator_->ProcessMessage(message, base::Bind(
         &NegotiatingAuthenticatorBase::UpdateState,
         base::Unretained(this), resume_callback));
diff --git a/remoting/protocol/negotiating_authenticator_unittest.cc b/remoting/protocol/negotiating_authenticator_unittest.cc
index 9b5bb8b..8a18f2d 100644
--- a/remoting/protocol/negotiating_authenticator_unittest.cc
+++ b/remoting/protocol/negotiating_authenticator_unittest.cc
@@ -12,6 +12,7 @@
 #include "remoting/protocol/negotiating_client_authenticator.h"
 #include "remoting/protocol/negotiating_host_authenticator.h"
 #include "remoting/protocol/pairing_registry.h"
+#include "remoting/protocol/protocol_mock_objects.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/libjingle/source/talk/xmllite/xmlelement.h"
@@ -54,13 +55,12 @@ class NegotiatingAuthenticatorTest : public AuthenticatorTestBase {
       const std::string& client_interactive_pin,
       const std::string& host_secret,
       AuthenticationMethod::HashFunction hash_function,
-      bool client_hmac_only,
-      scoped_refptr<PairingRegistry> pairing_registry) {
+      bool client_hmac_only) {
     std::string host_secret_hash = AuthenticationMethod::ApplyHashFunction(
         hash_function, kTestHostId, host_secret);
     host_ = NegotiatingHostAuthenticator::CreateWithSharedSecret(
         host_cert_, key_pair_, host_secret_hash, hash_function,
-        pairing_registry);
+        pairing_registry_);
 
     std::vector<AuthenticationMethod> methods;
     methods.push_back(AuthenticationMethod::Spake2Pair());
@@ -70,7 +70,7 @@ class NegotiatingAuthenticatorTest : public AuthenticatorTestBase {
       methods.push_back(AuthenticationMethod::Spake2(
           AuthenticationMethod::NONE));
     }
-    bool pairing_expected = pairing_registry.get() != NULL;
+    bool pairing_expected = pairing_registry_.get() != NULL;
     FetchSecretCallback fetch_secret_callback =
         base::Bind(&NegotiatingAuthenticatorTest::FetchSecret,
                    client_interactive_pin,
@@ -82,18 +82,16 @@ class NegotiatingAuthenticatorTest : public AuthenticatorTestBase {
     client_.reset(client_as_negotiating_authenticator_);
   }
 
-  scoped_refptr<PairingRegistry> CreatePairingRegistry(
-      PairingRegistry::Pairing* pairings, size_t num_pairings) {
-    PairingRegistry::PairedClients clients;
-    for (size_t i = 0; i < num_pairings; ++i) {
-      clients[pairings[i].client_id] = pairings[i];
+  void CreatePairingRegistry(bool with_paired_client) {
+    mock_delegate_ = new MockPairingRegistryDelegate;
+    if (with_paired_client) {
+      PairingRegistry::Pairing pairing;
+      pairing.client_id = kTestClientId;
+      pairing.shared_secret = kTestPairedSecret;
+      mock_delegate_->AddPairing(pairing);
     }
-    scoped_refptr<PairingRegistry> result(
-        new PairingRegistry(
-            scoped_ptr<PairingRegistry::Delegate>(
-                new NotImplementedPairingRegistryDelegate),
-            clients));
-    return result;
+    pairing_registry_ = new PairingRegistry(
+        scoped_ptr<PairingRegistry::Delegate>(mock_delegate_));
   }
 
   static void FetchSecret(
@@ -143,14 +141,19 @@ class NegotiatingAuthenticatorTest : public AuthenticatorTestBase {
   // Use a bare pointer because the storage is managed by the base class.
   NegotiatingClientAuthenticator* client_as_negotiating_authenticator_;
 
+ protected:
+  MockPairingRegistryDelegate* mock_delegate_;
+
  private:
+  scoped_refptr<PairingRegistry> pairing_registry_;
+
   DISALLOW_COPY_AND_ASSIGN(NegotiatingAuthenticatorTest);
 };
 
 TEST_F(NegotiatingAuthenticatorTest, SuccessfulAuthHmac) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kNoClientId, kNoPairedSecret, kTestPin, kTestPin,
-      AuthenticationMethod::HMAC_SHA256, false, NULL));
+      AuthenticationMethod::HMAC_SHA256, false));
   VerifyAccepted(
       AuthenticationMethod::Spake2(AuthenticationMethod::HMAC_SHA256));
 }
@@ -158,14 +161,14 @@ TEST_F(NegotiatingAuthenticatorTest, SuccessfulAuthHmac) {
 TEST_F(NegotiatingAuthenticatorTest, SuccessfulAuthPlain) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kNoClientId, kNoPairedSecret, kTestPin, kTestPin,
-      AuthenticationMethod::NONE, false, NULL));
+      AuthenticationMethod::NONE, false));
   VerifyAccepted(AuthenticationMethod::Spake2(AuthenticationMethod::NONE));
 }
 
 TEST_F(NegotiatingAuthenticatorTest, InvalidSecretHmac) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kNoClientId, kNoPairedSecret, kTestPinBad, kTestPin,
-      AuthenticationMethod::HMAC_SHA256, false, NULL));
+      AuthenticationMethod::HMAC_SHA256, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
 
   VerifyRejected(Authenticator::INVALID_CREDENTIALS);
@@ -174,7 +177,7 @@ TEST_F(NegotiatingAuthenticatorTest, InvalidSecretHmac) {
 TEST_F(NegotiatingAuthenticatorTest, InvalidSecretPlain) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kNoClientId, kNoPairedSecret, kTestPin, kTestPinBad,
-      AuthenticationMethod::NONE, false, NULL));
+      AuthenticationMethod::NONE, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
 
   VerifyRejected(Authenticator::INVALID_CREDENTIALS);
@@ -183,7 +186,7 @@ TEST_F(NegotiatingAuthenticatorTest, InvalidSecretPlain) {
 TEST_F(NegotiatingAuthenticatorTest, IncompatibleMethods) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kNoClientId, kNoPairedSecret, kTestPin, kTestPinBad,
-      AuthenticationMethod::NONE, true, NULL));
+      AuthenticationMethod::NONE, true));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
 
   VerifyRejected(Authenticator::PROTOCOL_ERROR);
@@ -192,72 +195,68 @@ TEST_F(NegotiatingAuthenticatorTest, IncompatibleMethods) {
 TEST_F(NegotiatingAuthenticatorTest, PairingNotSupported) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kTestClientId, kTestPairedSecret, kTestPin, kTestPin,
-      AuthenticationMethod::HMAC_SHA256, false, NULL));
+      AuthenticationMethod::HMAC_SHA256, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
   VerifyAccepted(
       AuthenticationMethod::Spake2(AuthenticationMethod::HMAC_SHA256));
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingSupportedButNotPaired) {
+  CreatePairingRegistry(false);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kNoClientId, kNoPairedSecret, kTestPin, kTestPin,
-      AuthenticationMethod::HMAC_SHA256, false,
-      CreatePairingRegistry(NULL, 0)));
+      AuthenticationMethod::HMAC_SHA256, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
   VerifyAccepted(AuthenticationMethod::Spake2Pair());
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingRevokedPinOkay) {
+  CreatePairingRegistry(false);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kTestClientId, kTestPairedSecret, kTestPin, kTestPin,
-      AuthenticationMethod::HMAC_SHA256, false,
-      CreatePairingRegistry(NULL, 0)));
+      AuthenticationMethod::HMAC_SHA256, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
+  mock_delegate_->RunCallback();
   VerifyAccepted(AuthenticationMethod::Spake2Pair());
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingRevokedPinBad) {
+  CreatePairingRegistry(false);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kTestClientId, kTestPairedSecret, kTestPinBad, kTestPin,
-      AuthenticationMethod::HMAC_SHA256, false,
-      CreatePairingRegistry(NULL, 0)));
+      AuthenticationMethod::HMAC_SHA256, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
+  mock_delegate_->RunCallback();
   VerifyRejected(Authenticator::INVALID_CREDENTIALS);
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingSucceeded) {
-  PairingRegistry::Pairing pairing;
-  pairing.client_id = kTestClientId;
-  pairing.shared_secret = kTestPairedSecret;
+  CreatePairingRegistry(true);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kTestClientId, kTestPairedSecret, kTestPinBad, kTestPin,
-      AuthenticationMethod::HMAC_SHA256, false,
-      CreatePairingRegistry(&pairing, 1)));
+      AuthenticationMethod::HMAC_SHA256, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
+  mock_delegate_->RunCallback();
   VerifyAccepted(AuthenticationMethod::Spake2Pair());
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingSucceededInvalidSecretButPinOkay) {
-  PairingRegistry::Pairing pairing;
-  pairing.client_id = kTestClientId;
-  pairing.shared_secret = kTestPairedSecret;
+  CreatePairingRegistry(true);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kTestClientId, kTestPairedSecretBad, kTestPin, kTestPin,
-      AuthenticationMethod::HMAC_SHA256, false,
-      CreatePairingRegistry(&pairing, 1)));
+      AuthenticationMethod::HMAC_SHA256, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
+  mock_delegate_->RunCallback();
   VerifyAccepted(AuthenticationMethod::Spake2Pair());
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingFailedInvalidSecretAndPin) {
-  PairingRegistry::Pairing pairing;
-  pairing.client_id = kTestClientId;
-  pairing.shared_secret = kTestPairedSecret;
+  CreatePairingRegistry(true);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kTestClientId, kTestPairedSecretBad, kTestPinBad, kTestPin,
-      AuthenticationMethod::HMAC_SHA256, false,
-      CreatePairingRegistry(&pairing, 1)));
+      AuthenticationMethod::HMAC_SHA256, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
+  mock_delegate_->RunCallback();
   VerifyRejected(Authenticator::INVALID_CREDENTIALS);
 }
 
diff --git a/remoting/protocol/pairing_host_authenticator.cc b/remoting/protocol/pairing_host_authenticator.cc
index 9e57265..75db3a7a 100644
--- a/remoting/protocol/pairing_host_authenticator.cc
+++ b/remoting/protocol/pairing_host_authenticator.cc
@@ -9,7 +9,6 @@
 #include "remoting/base/constants.h"
 #include "remoting/base/rsa_key_pair.h"
 #include "remoting/protocol/channel_authenticator.h"
-#include "remoting/protocol/pairing_registry.h"
 #include "remoting/protocol/v2_authenticator.h"
 #include "third_party/libjingle/source/talk/xmllite/xmlelement.h"
 
@@ -26,6 +25,7 @@ PairingHostAuthenticator::PairingHostAuthenticator(
       key_pair_(key_pair),
       pin_(pin),
       protocol_error_(false),
+      waiting_for_paired_secret_(false),
       weak_factory_(this) {
 }
 
@@ -35,6 +35,8 @@ PairingHostAuthenticator::~PairingHostAuthenticator() {
 Authenticator::State PairingHostAuthenticator::state() const {
   if (protocol_error_) {
     return REJECTED;
+  } else if (waiting_for_paired_secret_) {
+    return PROCESSING_MESSAGE;
   } else if (!v2_authenticator_) {
     return WAITING_MESSAGE;
   }
@@ -72,23 +74,13 @@ void PairingHostAuthenticator::ProcessMessage(
       LOG(ERROR) << "No client id specified.";
       protocol_error_ = true;
     } else {
-      paired_secret = pairing_registry_->GetSecret(client_id);
-      if (paired_secret.empty()) {
-        LOG(INFO) << "Unknown client id";
-        error_message_ = "unknown-client-id";
-      }
-    }
-
-    using_paired_secret_ = !paired_secret.empty();
-    if (using_paired_secret_) {
-      v2_authenticator_  = V2Authenticator::CreateForHost(
-          local_cert_, key_pair_, paired_secret, WAITING_MESSAGE);
-    } else {
-      v2_authenticator_ = V2Authenticator::CreateForHost(
-          local_cert_, key_pair_, pin_, MESSAGE_READY);
-      // The client's optimistic SPAKE message is using a Paired Secret to
-      // which the host doesn't have access, so don't bother processing it.
-      resume_callback.Run();
+      waiting_for_paired_secret_ = true;
+      pairing_registry_->GetPairing(
+          client_id,
+          base::Bind(&PairingHostAuthenticator::ProcessMessageWithPairing,
+                     weak_factory_.GetWeakPtr(),
+                     base::Owned(new buzz::XmlElement(*message)),
+                     resume_callback));
       return;
     }
   }
@@ -100,5 +92,30 @@ void PairingHostAuthenticator::AddPairingElements(buzz::XmlElement* message) {
   // Nothing to do here
 }
 
+void PairingHostAuthenticator::ProcessMessageWithPairing(
+    const buzz::XmlElement* message,
+    const base::Closure& resume_callback,
+    PairingRegistry::Pairing pairing) {
+  waiting_for_paired_secret_ = false;
+  std::string paired_secret = pairing.shared_secret;
+  if (paired_secret.empty()) {
+    LOG(INFO) << "Unknown client id";
+    error_message_ = "unknown-client-id";
+  }
+
+  using_paired_secret_ = !paired_secret.empty();
+  if (using_paired_secret_) {
+    v2_authenticator_  = V2Authenticator::CreateForHost(
+        local_cert_, key_pair_, paired_secret, WAITING_MESSAGE);
+    PairingAuthenticatorBase::ProcessMessage(message, resume_callback);
+  } else {
+    v2_authenticator_ = V2Authenticator::CreateForHost(
+        local_cert_, key_pair_, pin_, MESSAGE_READY);
+    // The client's optimistic SPAKE message is using a Paired Secret to
+    // which the host doesn't have access, so don't bother processing it.
+    resume_callback.Run();
+  }
+}
+
 }  // namespace protocol
 }  // namespace remoting
diff --git a/remoting/protocol/pairing_host_authenticator.h b/remoting/protocol/pairing_host_authenticator.h
index a4884c4..57c900c 100644
--- a/remoting/protocol/pairing_host_authenticator.h
+++ b/remoting/protocol/pairing_host_authenticator.h
@@ -7,6 +7,7 @@
 
 #include "base/memory/weak_ptr.h"
 #include "remoting/protocol/pairing_authenticator_base.h"
+#include "remoting/protocol/pairing_registry.h"
 
 namespace remoting {
 
@@ -38,12 +39,19 @@ class PairingHostAuthenticator : public PairingAuthenticatorBase {
       const SetAuthenticatorCallback& callback) OVERRIDE;
   virtual void AddPairingElements(buzz::XmlElement* message) OVERRIDE;
 
+  // Continue processing a protocol message once the pairing information for
+  // the client id has been received.
+  void ProcessMessageWithPairing(const buzz::XmlElement* message,
+                                 const base::Closure& resume_callback,
+                                 PairingRegistry::Pairing pairing);
+
   // Protocol state.
   scoped_refptr<PairingRegistry> pairing_registry_;
   std::string local_cert_;
   scoped_refptr<RsaKeyPair> key_pair_;
   const std::string& pin_;
   bool protocol_error_;
+  bool waiting_for_paired_secret_;
 
   base::WeakPtrFactory<PairingHostAuthenticator> weak_factory_;
 
diff --git a/remoting/protocol/pairing_registry.cc b/remoting/protocol/pairing_registry.cc
index 54b66b0..10098b0 100644
--- a/remoting/protocol/pairing_registry.cc
+++ b/remoting/protocol/pairing_registry.cc
@@ -15,17 +15,15 @@ namespace protocol {
 // How many bytes of random data to use for the client id and shared secret.
 const int kKeySize = 16;
 
-PairingRegistry::PairingRegistry(scoped_ptr<Delegate> delegate,
-                                 const PairedClients& paired_clients)
+PairingRegistry::PairingRegistry(scoped_ptr<Delegate> delegate)
     : delegate_(delegate.Pass()) {
   DCHECK(delegate_);
-  paired_clients_ = paired_clients;
 }
 
 PairingRegistry::~PairingRegistry() {
 }
 
-const PairingRegistry::Pairing& PairingRegistry::CreatePairing(
+PairingRegistry::Pairing PairingRegistry::CreatePairing(
     const std::string& client_name) {
   DCHECK(CalledOnValidThread());
 
@@ -42,27 +40,28 @@ const PairingRegistry::Pairing& PairingRegistry::CreatePairing(
   }
 
   // Save the result via the Delegate and return it to the caller.
-  paired_clients_[result.client_id] = result;
-  delegate_->Save(paired_clients_);
-
-  return paired_clients_[result.client_id];
+  delegate_->AddPairing(result);
+  return result;
 }
 
-std::string PairingRegistry::GetSecret(const std::string& client_id) const {
+void PairingRegistry::GetPairing(const std::string& client_id,
+                                 const GetPairingCallback& callback) {
   DCHECK(CalledOnValidThread());
+  delegate_->GetPairing(client_id, callback);
+}
 
-  std::string result;
-  PairedClients::const_iterator i = paired_clients_.find(client_id);
-  if (i != paired_clients_.end()) {
-    result = i->second.shared_secret;
-  }
-  return result;
+void NotImplementedPairingRegistryDelegate::AddPairing(
+    const PairingRegistry::Pairing& new_paired_client) {
+  NOTIMPLEMENTED();
 }
 
-void NotImplementedPairingRegistryDelegate::Save(
-    const PairingRegistry::PairedClients& paired_clients) {
+void NotImplementedPairingRegistryDelegate::GetPairing(
+    const std::string& client_id,
+    const PairingRegistry::GetPairingCallback& callback) {
   NOTIMPLEMENTED();
+  callback.Run(PairingRegistry::Pairing());
 }
 
+
 }  // namespace protocol
 }  // namespace remoting
diff --git a/remoting/protocol/pairing_registry.h b/remoting/protocol/pairing_registry.h
index f96097b..c32c54f 100644
--- a/remoting/protocol/pairing_registry.h
+++ b/remoting/protocol/pairing_registry.h
@@ -37,24 +37,33 @@ class PairingRegistry : public base::RefCountedThreadSafe<PairingRegistry>,
   // Mapping from client id to pairing information.
   typedef std::map<std::string, Pairing> PairedClients;
 
+  // Delegate::GetPairing callback.
+  typedef base::Callback<void(Pairing)> GetPairingCallback;
+
   // Interface representing the persistent storage back-end.
   class Delegate {
    public:
     virtual ~Delegate() {}
 
-    // Save pairing information to persistent storage. Must not block.
-    virtual void Save(const PairedClients& paired_clients) = 0;
+    // Add pairing information to persistent storage. Must not block.
+    virtual void AddPairing(const Pairing& new_paired_client) = 0;
+
+    // Retrieve the Pairing for the specified client id. If none is
+    // found, invoke the callback with a Pairing in which (at least)
+    // the shared_secret is empty.
+    virtual void GetPairing(const std::string& client_id,
+                            const GetPairingCallback& callback) = 0;
   };
 
-  explicit PairingRegistry(scoped_ptr<Delegate> delegate,
-                           const PairedClients& paired_clients);
+  explicit PairingRegistry(scoped_ptr<Delegate> delegate);
 
   // Create a pairing for a new client and save it to disk.
-  const Pairing& CreatePairing(const std::string& client_name);
+  Pairing CreatePairing(const std::string& client_name);
 
-  // Look up the shared secret for the specified client id. Returns an empty
-  // string if the client id is not known.
-  std::string GetSecret(const std::string &client_id) const;
+  // Get the pairing for the specified client id. See the corresponding
+  // Delegate method for details.
+  void GetPairing(const std::string& client_id,
+                  const GetPairingCallback& callback);
 
  private:
   friend class base::RefCountedThreadSafe<PairingRegistry>;
@@ -62,7 +71,6 @@ class PairingRegistry : public base::RefCountedThreadSafe<PairingRegistry>,
   virtual ~PairingRegistry();
 
   scoped_ptr<Delegate> delegate_;
-  PairedClients paired_clients_;
 
   DISALLOW_COPY_AND_ASSIGN(PairingRegistry);
 };
@@ -71,8 +79,11 @@ class PairingRegistry : public base::RefCountedThreadSafe<PairingRegistry>,
 // TODO(jamiewalch): Delete once Delegates are implemented for all platforms.
 class NotImplementedPairingRegistryDelegate : public PairingRegistry::Delegate {
  public:
-  virtual void Save(
-      const PairingRegistry::PairedClients& paired_clients) OVERRIDE;
+  virtual void AddPairing(
+      const PairingRegistry::Pairing& paired_clients) OVERRIDE;
+  virtual void GetPairing(
+      const std::string& client_id,
+      const PairingRegistry::GetPairingCallback& callback) OVERRIDE;
 };
 
 }  // namespace protocol
diff --git a/remoting/protocol/pairing_registry_unittest.cc b/remoting/protocol/pairing_registry_unittest.cc
index fcab43a..523bdaa 100644
--- a/remoting/protocol/pairing_registry_unittest.cc
+++ b/remoting/protocol/pairing_registry_unittest.cc
@@ -6,7 +6,9 @@
 
 #include <stdlib.h>
 
+#include "base/bind.h"
 #include "base/compiler_specific.h"
+#include "remoting/protocol/protocol_mock_objects.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -14,55 +16,56 @@ namespace remoting {
 namespace protocol {
 
 class PairingRegistryTest : public testing::Test {
-};
-
-class MockDelegate : public PairingRegistry::Delegate {
  public:
-  // MockDelegate saves to an explicit external PairedClients instance because
-  // PairingRegistry takes ownership of it and makes no guarantees about its
-  // lifetime, so this is a simple way of getting access to pairing results.
-  MockDelegate(PairingRegistry::PairedClients* paired_clients)
-      : paired_clients_(paired_clients) {
+  virtual void SetUp() OVERRIDE {
+    got_secret_ = false;
   }
 
-  virtual void Save(
-      const PairingRegistry::PairedClients& paired_clients) OVERRIDE {
-    *paired_clients_ = paired_clients;
+  void CompareSecret(const std::string& expected,
+                     PairingRegistry::Pairing actual) {
+    EXPECT_EQ(actual.shared_secret, expected);
+    got_secret_ = true;
   }
 
  protected:
-  PairingRegistry::PairedClients* paired_clients_;
+  bool got_secret_;
 };
 
-TEST_F(PairingRegistryTest, LoadAndLookup) {
+TEST_F(PairingRegistryTest, GetPairing) {
   PairingRegistry::Pairing client_info = {
     "client_id",
     "client_name",
     "shared_secret"
   };
-  PairingRegistry::PairedClients clients;
-  clients[client_info.client_id] = client_info;
-  scoped_ptr<PairingRegistry::Delegate> delegate(new MockDelegate(&clients));
-
-  scoped_refptr<PairingRegistry> registry(new PairingRegistry(delegate.Pass(),
-                                                              clients));
-
-  std::string secret = registry->GetSecret(client_info.client_id);
-  EXPECT_EQ(secret, client_info.shared_secret);
+  MockPairingRegistryDelegate* mock_delegate =
+      new MockPairingRegistryDelegate();
+  mock_delegate->AddPairing(client_info);
+  scoped_ptr<PairingRegistry::Delegate> delegate(mock_delegate);
+
+  scoped_refptr<PairingRegistry> registry(new PairingRegistry(delegate.Pass()));
+
+  registry->GetPairing(client_info.client_id,
+                       base::Bind(&PairingRegistryTest::CompareSecret,
+                                  base::Unretained(this),
+                                  client_info.shared_secret));
+  mock_delegate->RunCallback();
+  EXPECT_TRUE(got_secret_);
 }
 
-TEST_F(PairingRegistryTest, CreateAndSave) {
-  PairingRegistry::PairedClients clients;
-  scoped_ptr<PairingRegistry::Delegate> delegate(new MockDelegate(&clients));
+TEST_F(PairingRegistryTest, AddPairing) {
+  MockPairingRegistryDelegate* mock_delegate =
+      new MockPairingRegistryDelegate();
+  scoped_ptr<PairingRegistry::Delegate> delegate(mock_delegate);
 
-  scoped_refptr<PairingRegistry> registry(new PairingRegistry(delegate.Pass(),
-                                                              clients));
+  scoped_refptr<PairingRegistry> registry(new PairingRegistry(delegate.Pass()));
 
   // Verify that we can create pairings from two clients with the same name, but
   // that they aren't allocated the same client id.
   PairingRegistry::Pairing pairing_1 = registry->CreatePairing("client_name");
   PairingRegistry::Pairing pairing_2 = registry->CreatePairing("client_name");
 
+  const PairingRegistry::PairedClients& clients =
+      mock_delegate->paired_clients();
   ASSERT_EQ(clients.size(), 2u);
   PairingRegistry::Pairing first = clients.begin()->second;
   PairingRegistry::Pairing second = (++clients.begin())->second;
diff --git a/remoting/protocol/protocol_mock_objects.cc b/remoting/protocol/protocol_mock_objects.cc
index 072cf3c..eb4022a 100644
--- a/remoting/protocol/protocol_mock_objects.cc
+++ b/remoting/protocol/protocol_mock_objects.cc
@@ -4,10 +4,6 @@
 
 #include "remoting/protocol/protocol_mock_objects.h"
 
-#include "base/message_loop/message_loop_proxy.h"
-#include "net/base/ip_endpoint.h"
-#include "remoting/protocol/transport.h"
-
 namespace remoting {
 namespace protocol {
 
@@ -52,5 +48,34 @@ MockSessionManager::MockSessionManager() {}
 
 MockSessionManager::~MockSessionManager() {}
 
+MockPairingRegistryDelegate::MockPairingRegistryDelegate() {
+}
+
+MockPairingRegistryDelegate::~MockPairingRegistryDelegate() {
+}
+
+void MockPairingRegistryDelegate::AddPairing(
+    const PairingRegistry::Pairing& new_paired_client) {
+  paired_clients_[new_paired_client.client_id] = new_paired_client;
+}
+
+void MockPairingRegistryDelegate::GetPairing(
+    const std::string& client_id,
+    const PairingRegistry::GetPairingCallback& callback) {
+  PairingRegistry::Pairing result;
+  PairingRegistry::PairedClients::const_iterator i =
+      paired_clients_.find(client_id);
+  if (i != paired_clients_.end()) {
+    result = i->second;
+  }
+  saved_callback_ = base::Bind(base::Bind(callback), result);
+}
+
+void MockPairingRegistryDelegate::RunCallback() {
+  DCHECK(!saved_callback_.is_null());
+  saved_callback_.Run();
+  saved_callback_.Reset();
+}
+
 }  // namespace protocol
 }  // namespace remoting
diff --git a/remoting/protocol/protocol_mock_objects.h b/remoting/protocol/protocol_mock_objects.h
index 54dc150..7262ae5 100644
--- a/remoting/protocol/protocol_mock_objects.h
+++ b/remoting/protocol/protocol_mock_objects.h
@@ -16,6 +16,7 @@
 #include "remoting/protocol/connection_to_client.h"
 #include "remoting/protocol/host_stub.h"
 #include "remoting/protocol/input_stub.h"
+#include "remoting/protocol/pairing_registry.h"
 #include "remoting/protocol/session.h"
 #include "remoting/protocol/session_manager.h"
 #include "remoting/protocol/transport.h"
@@ -203,6 +204,30 @@ class MockSessionManager : public SessionManager {
   DISALLOW_COPY_AND_ASSIGN(MockSessionManager);
 };
 
+// Simple delegate that caches information on paired clients in memory.
+class MockPairingRegistryDelegate : public PairingRegistry::Delegate {
+ public:
+  MockPairingRegistryDelegate();
+  virtual ~MockPairingRegistryDelegate();
+
+  const PairingRegistry::PairedClients& paired_clients() const {
+    return paired_clients_;
+  }
+
+  // PairingRegistry::Delegate implementation.
+  virtual void AddPairing(
+      const PairingRegistry::Pairing& new_paired_client) OVERRIDE;
+  virtual void GetPairing(
+      const std::string& client_id,
+      const PairingRegistry::GetPairingCallback& callback) OVERRIDE;
+
+  void RunCallback();
+
+ private:
+  base::Closure saved_callback_;
+  PairingRegistry::PairedClients paired_clients_;
+};
+
 }  // namespace protocol
 }  // namespace remoting