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..5e41beb5ab3365e4ac9539497076b5e5fbfd314a
--- /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;
+
+namespace protocol {
+
+class Authenticator;
+
+typedef base::Callback<void(const std::string& secret)> SecretFetchedCallback;
+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

[Wez, 2013/05/18 20:08:36]

nit: Suggest capitalizing 'Client Id', so that when you refer to it in other
comments it stands out as a specific technical term, similarly to 'Access Code',
'Shared Secret', 'Host Secret', etc elsewhere.

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

Done.

+//   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.
+//

[Wez, 2013/05/18 20:08:36]

nit: Lose the extra blank comment

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

Done.

+//
+// If a client device is not already paired, but supports pairing, then
+// the V2Authenticator is used instead of this class. Only the method name
+// differs, which the client uses to determine that pairing should be offered
+// to the user.
+class PairingClientAuthenticator : public Authenticator {
+ 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.

[Wez, 2013/05/18 20:08:36]

This is a little confusing; are you saying that in both the ACCEPTED and
REJECTED |pairing_state_| we defer overall to the V2 authenticator's state?

Suggest something like:
Stores the progress of the pairing exchange. During the pairing exchange this
corresponds to the state of the authentication exchange as a whole. Once pairing
is ACCEPTED or REJECTED then the underlying V2 authenticator determines the
overall state.

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

I've used a couple of booleans in the new implementation, which I think makes
the code more readable.

+  State pairing_state_;
+
+  std::string client_id_;

[Wez, 2013/05/18 20:08:36]

nit: Please add at least a block comment for these members. :)

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

Done.

+  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_;

[Wez, 2013/05/18 20:08:36]

nit: Suggest a blank line preceding |weak_factory_|, so it stands out from the
block of other members.

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

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(PairingClientAuthenticator);
+};
+
+}  // namespace protocol
+}  // namespace remoting
+
+#endif  // REMOTING_PROTOCOL_PAIRING_AUTHENTICATOR_H_