PairingAuthenticator implementation and plumbing.

remoting/protocol/pairing_client_authenticator.h

Index: remoting/protocol/pairing_client_authenticator.h
diff --git a/remoting/protocol/pairing_client_authenticator.h b/remoting/protocol/pairing_client_authenticator.h
new file mode 100644
index 0000000000000000000000000000000000000000..e52c0d4789bf76080d840e229815343d6e4e9dd2
--- /dev/null
+++ b/remoting/protocol/pairing_client_authenticator.h
@@ -0,0 +1,89 @@
+// Copyright 2013 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 REMOTING_PROTOCOL_PAIRING_CLIENT_AUTHENTICATOR_H_
+#define REMOTING_PROTOCOL_PAIRING_CLIENT_AUTHENTICATOR_H_
+
+#include "base/memory/weak_ptr.h"
+#include "remoting/protocol/authenticator.h"
+
+namespace remoting {
+
+class RsaKeyPair;

[Sergey Ulanov, 2013/05/17 01:57:17]

nit: Not used.

[Jamie, 2013/05/21 01:24:33]

Done.

+
+namespace protocol {
+
+class Authenticator;
+
+typedef base::Callback<void(const std::string& secret)> SecretFetchedCallback;

[Sergey Ulanov, 2013/05/17 01:57:17]

These are also declared in negotiating_client_authenticator.h.  Maybe move them
somewhere to avoid duplication (e.g. authenticator.h). Also I think they should
be called AuthSecretFetchedCallback and FetchAuthSecretCallback. Or maybe just
put them inside Authenticator interface?

[rmsousa, 2013/05/17 20:09:03]

They are actually the same function as the regular FetchSecretCallback used for
the PIN. But I agree they should be imported from a common place in that case
(authenticator.h is a good suggestion).

[Jamie, 2013/05/21 01:24:33]

I've moved them to authenticator, but I've left the names as they are now to
avoid changing more code than I need to in this CL.

+typedef base::Callback<void(
+    const SecretFetchedCallback& secret_fetched_callback)> FetchSecretCallback;
+
+// PairingClientAuthenticator builds on top of V2Authenticator to add
+// support for PIN-less authentication via device pairing:
+//
+// * If a client device is already paired, it includes a client id in
+//   the initial authentication message.
+// * If the host recognizes the id, it looks up the corresponding
+//   paired secret and initiates a SPAKE with HMAC_SHA256.
+// * If it does not recognize the id, it initiates a SPAKE exchange
+//   with HMAC_SHA256 using the PIN as the shared secret. The initial
+//   message of this exchange includes an an error message, which
+//   informs the client that the PIN-less connection failed and causes
+//   it to prompt the user for a PIN to use for authentication
+//   instead.
+//
+// If a client device is not already paired, but supports pairing, it
+// sends an empty initial authentication message. In this case, the
+// authenticator behaves exactly like the V2Authenticator with
+// HMAC_SHA256--only the method name differs, which the client uses to
+// detect that pairing should be offered to the user.
+class PairingClientAuthenticator : public Authenticator {

[Sergey Ulanov, 2013/05/17 01:57:17]

Can you please add some unittests for the new authenticator. It's important that
we have unittests for all authenticators. Having many bugs here would be
dangerous. I saw that you added unittests for negotiating authenticator, but I
think we should also have unittests for this code without the negotiating layer.

[Jamie, 2013/05/21 01:24:33]

The tests in negotiating_authenticator_unittests are actually pretty
comprehensive, so I'm not very concerned about coverage. I agree that it would
be nice to test the pairing classes independently, but do you mind if I wait
until the rest of this feature has landed?

+ public:
+  PairingClientAuthenticator(
+      const std::string& client_id,
+      const std::string& paired_secret,
+      const FetchSecretCallback& fetch_pin_callback,
+      const std::string& authentication_tag);
+  virtual ~PairingClientAuthenticator() {}
+
+  // Authenticator interface.
+  virtual State state() const OVERRIDE;
+  virtual RejectionReason rejection_reason() const OVERRIDE;
+  virtual void ProcessMessage(const buzz::XmlElement* message,
+                              const base::Closure& resume_callback) OVERRIDE;
+  virtual scoped_ptr<buzz::XmlElement> GetNextMessage() OVERRIDE;
+  virtual scoped_ptr<ChannelAuthenticator>
+      CreateChannelAuthenticator() const OVERRIDE;
+
+ private:
+  void CreateV2AuthenticatorWithPIN(const buzz::XmlElement* message,
+                                    const base::Closure& resume_callback,
+                                    const std::string& pin);
+
+  // |pairing_state_| indicates the progress of the initial pairing client
+  // id exchange. MESSAGE_READY, WAITING_MESSAGE and PROCESSING_MESSAGE map
+  // directly to the corresponding return value from the |state| method (the
+  // latter means that the user is being prompted for the PIN). ACCEPTED and
+  // REJECTED mean that the client id exchange is complete and succeeded or
+  // failed, respectively. Currently, no distinction is made between these
+  // two states--in either case the underlying v2 authenticator has been
+  // created and holds the state of the overall auth exchange.
+  State pairing_state_;
+
+  std::string client_id_;
+  const std::string& paired_secret_;
+  FetchSecretCallback fetch_pin_callback_;
+  std::string authentication_tag_;
+  State initial_state_;
+  scoped_ptr<Authenticator> v2_authenticator_;
+  base::WeakPtrFactory<PairingClientAuthenticator> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(PairingClientAuthenticator);
+};
+
+}  // namespace protocol
+}  // namespace remoting
+
+#endif  // REMOTING_PROTOCOL_PAIRING_AUTHENTICATOR_H_