PairingAuthenticator implementation and plumbing.

remoting/protocol/pairing_host_authenticator.h

Index: remoting/protocol/pairing_host_authenticator.h
diff --git a/remoting/protocol/pairing_host_authenticator.h b/remoting/protocol/pairing_host_authenticator.h
new file mode 100644
index 0000000000000000000000000000000000000000..c5102f835910173b860f3baeada85356219618a2
--- /dev/null
+++ b/remoting/protocol/pairing_host_authenticator.h
@@ -0,0 +1,76 @@
+// 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_HOST_AUTHENTICATOR_H_
+#define REMOTING_PROTOCOL_PAIRING_HOST_AUTHENTICATOR_H_
+
+#include "base/memory/weak_ptr.h"
+#include "remoting/protocol/authenticator.h"
+
+namespace remoting {
+
+class RsaKeyPair;
+
+namespace protocol {
+
+class V2Authenticator;
+class PairingRegistry;
+
+// PairingHostAuthenticator 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: See comment in client imp re capitalization.

[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.
+//
+// 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 PairingHostAuthenticator : public Authenticator {
+ public:
+  PairingHostAuthenticator(
+      scoped_refptr<PairingRegistry> pairing_registry,
+      const std::string& local_cert,
+      scoped_refptr<RsaKeyPair> key_pair,
+      const std::string& pin);
+  virtual ~PairingHostAuthenticator() {}
+
+  // 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();
+  void CheckForFailedSpakeExchange(const base::Closure& resume_callback);

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

nit: Comments for these members, please.

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

Done in the base class for the former. The latter is now just one of several
small helper functions. I don't think they all need comments.

[Wez, 2013/05/22 00:51:45]

Style guide says otherwise. ;) But yeah, you can probably get away without
comments on each, if you instead have some comment e.g. "Helper functions for
blah blah blah" for the block of methods.

[Jamie, 2013/05/22 01:16:46]

Done.

+
+  scoped_refptr<PairingRegistry> pairing_registry_;

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

nit: At least a block comment introducing these members, please.

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

Done.

+  std::string local_cert_;
+  scoped_refptr<RsaKeyPair> key_pair_;
+  const std::string& pin_;
+  scoped_ptr<Authenticator> v2_authenticator_;
+  std::string error_message_;
+  bool protocol_error_;

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

nit: This name sounds like it stores an error, rather than just being an
indicator that one occurred. Do you actually need it, or could you test for
|error_message_| being empty?

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

error_message doesn't indicate a fatal error, just a failure to authenticate
using paired credentials. protocol_error is fatal.

+  bool using_paired_secret_;
+  base::WeakPtrFactory<PairingHostAuthenticator> weak_factory_;

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

nit: Blank line before this, please.

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

Done.

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