Initial remoting::protocol::Authenticator interface.

remoting/protocol/authenticator.h

+// Copyright (c) 2011 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_AUTHENTICATOR_H_
+#define REMOTING_PROTOCOL_AUTHENTICATOR_H_
+
+#include <string>
+
+namespace buzz {
+class XmlElement;
+}  // namespace buzz
+
+namespace remoting {
+namespace protocol {
+
+// Authenticator is an abstract interface for authentication protocol
+// implementations. The same interface is used on both ends of the
+// connection, but it may be implemented differently on each side of
+// the connection.

[Wez, 2011/11/10 02:26:59]

I don't think the second sentence is necessary.

You might instead explain that the Chromoting client and host will repeatedly
call their Authenticators and deliver the messages they generate, until they
report successful authentication.

[Sergey Ulanov, 2011/11/10 23:17:47]

Done.

+//
+// Authenticator instances on connection ends may exchange multiple

[Wez, 2011/11/10 02:26:59]

reword: Authenticators may exchange...

[Sergey Ulanov, 2011/11/10 23:17:47]

Done.

+// messages before session is authenticated. Each message
+// sent/received by an Authenticator is delivered either in a session
+// description inside session-initiate and session-accept messages or
+// in a session-info message.

[Wez, 2011/11/10 02:26:59]

Explain why it is that some of the exchange uses one scheme, and the rest of it
uses another, rather than just always using session-info?

Are we really abusing session-info?

[Sergey Ulanov, 2011/11/10 23:17:47]

Done.
No. Session-info is designed for stuff like this.
http://xmpp.org/extensions/xep-0166.html#def-action-session-info :
  The session-info action is used to send session-level information, such as a
session ping or (for Jingle RTP sessions) a ringing message.
Not that it is different from transport-info messages.

+class Authenticator {
+ public:
+  // Allowed state transitions:
+  // When ProcessMessage() is called:
+  //    WAITING_MESSAGE -> MESSAGE_READY
+  //    WAITING_MESSAGE -> ACCEPTED
+  //    WAITING_MESSAGE -> REJECTED
+  // When GetNextMessage() is called:
+  //    MESSAGE_READY -> WAITING_MESSAGE
+  //    MESSAGE_READY -> ACCEPTED
+  //    MESSAGE_READY -> REJECTED
+  enum State {
+    // Waiting for the next message from the peer.
+    WAITING_MESSAGE,
+
+    // Next message is ready to be sent to the peer.
+    MESSAGE_READY,
+
+    // Session is authenticated successufully.
+    ACCEPTED,
+
+    // Session is rejected.
+    REJECTED,
+  };
+
+  Authenticator() {}
+  virtual ~Authenticator() {}
+
+  // Returns current state of the authenticator.
+  virtual State state() const = 0;
+
+  // Called in response to incoming message received from the peer.
+  // Should only be called when in WAITING_MESSAGE state.
+  virtual State ProcessMessage(talk_base::XmlElement* message) = 0;

[Wez, 2011/11/11 22:12:00]

For the existing IT2Me authentication scheme, we know whether we accept the
session by the time ProcessMessage() returns, but we still have to send a
message back to the peer, and the peer isn't going to send us another message to
process.

So I think we really need to separate State out from ProcessMessage() or
GetNextMessage(); the caller just keeps calling us and fetching another message
for ProcessMessage(), or calling GetNextMessage() and sending it, depending on
what state we're in, until the State is Accepted or Rejected?

[Sergey Ulanov, 2011/11/12 00:12:49]

I don't think there is a problem for the existing scheme. On the client side
Authenticator will go from MESSAGE_READY to ACCEPTED after first auth message is
sent with session-initiate. On the host side the Authenticator receives the
message and goes from WAITING_MESSAGE to ACCEPTED or REJECTED. Authenticator
doesn't need to worry about sending the session-accept message - Session will
need to do it even if the AUTHENTICATOR is in the ACCEPTED state.

[Wez, 2011/11/12 00:25:36]

Not sure I understand the last comment; do you mean that the Session will
*always* send a session-accept message?  I thought the SPAKE2-based auth was
going to use initiate->accept->info->info to exchange the messages, so if the
Session then sends an accept when it sees we've authenticated, won't that be a
duplicate?

+
+  // Must be called when in MESSAGE_READY state. Returns next
+  // authentication message that needs to be sent to the peer.
+  virtual talk_base::XmlElement* GetNextMessage() = 0;
+
+  // Following methods can be called only in the ACCEPTED state.
+
+  // Returns local SSL certificate that should be used for the
+  // session. Can return empty on the client side if client cert
+  // should not be used.
+  virtual std::string GetLocalCert() const = 0;
+
+  // Returns remote SSL certificate that should be used for the
+  // session. Can return empty on the host side if client cert
+  // should not be used.
+  virtual std::string GetPeerCert() const = 0;

[Wez, 2011/11/10 02:26:59]

Do we need GetLocalCert and GetPeerCert?  Isn't certificate verification
internal to an Authenticator?

e.g. a certificate-verifying Authenticator would be created by calling code and
supplied with a store of permitted certificates, and the Authenticator then
passed to the main Chromoting code, where it will set up channels and check the
certs against the store?

[Sergey Ulanov, 2011/11/10 23:17:47]

GetPeerCert() and GetLocalCert() return certs that should be used for SSL
connections of the channels. protocol::Session will use these to get certs when
it starts SSL handshake.
I think LocalCert is always specified when authentication is created or not
specified at all. Potentially it can be passed directly to Session() when it is
created, but I think it is cleaner when we keep cert-management in the
authenticator.
In the current IT2Me implementation as well as EKE-based auth the host cert is
received over signalling channel and it will be returned by GetPeerCert(). On
the host side GetPeerCert() would return empty string (i.e. client certs are
disabled).
For proper cert-based authentication we would have to replace GetPeerCert() with
VerifyPeerCert() function - but the problem is that it would not be usable with
our current SSL stack because it doesn't allow to use an arbitrary cert verifier
- net::CertVerifier is not an abstract interface. I've added TODO for this.

[Wez, 2011/11/11 01:17:42]

Let's discuss this offline; I think it's important to make it clear _how_ the
certs should be used, if we're specifying them here.

Note that the EKE-based authentication scheme doesn't require verification of
the host certificate.

[Sergey Ulanov, 2011/11/11 19:16:41]

Removed these methods for now.
 
It doesn't require it, but I think it is still useful to verify that host cert
matches what we receive from directory.

+
+  // Returns shared secret used to verify channels.
+  virtual std::string GetSharedSecret() const = 0;

[Sergey Ulanov, 2011/11/10 00:12:51]

Alternatively we can return ChannelAuthenticator here, but it would require some
refactoring of the ChannelAuthenticator interface, and I'm not sure if it is
necessary right now.

[Wez, 2011/11/10 02:26:59]

We'll need to be able to return different kinds of ChannelAuthenticator to
implement the EKE-based protocol.

[Sergey Ulanov, 2011/11/10 23:17:47]

Ok I've replaced it with CreateChannelAuthenticator()

+};
+
+// Factory for Authenticator instances.
+class AuthenticatorFactory {
+  // Called when session-initiate stanza is received to create
+  // authenticator for the new session. |first_message| specified

[Wez, 2011/11/10 02:26:59]

typo: specified -> specifies (or "provides")

[Sergey Ulanov, 2011/11/10 23:17:47]

Done.

+  // authentication part of the session-initiate stanza so that
+  // appropriate type of Authenticator can be choosed for the session

[Wez, 2011/11/10 02:26:59]

typo: choosed -> chosen

[Sergey Ulanov, 2011/11/10 23:17:47]

Done.

+  // (useful when multiple authenticators is supported). Returns NULL
+  // if the |first_message| is invalid and the session should be
+  // rejected. ProcessMessage() should be called with |first_message|
+  // for the result of this method.

[Wez, 2011/11/10 02:26:59]

Although the last line makes sense, it's an implementation detail not relevant
to the caller, I don't think.

Should the caller make sure to invoke GetNextMessage() and send whatever it
returns to the peer, if the Authenticator is non-NULL, though?

[Sergey Ulanov, 2011/11/10 23:17:47]

The idea is that the caller must call ProcessMessage(), it is not called inside
of CreateAuthenticator(). I.e. it is not an implementation detail. Reworder this
sentence to make it clearer.

[Wez, 2011/11/11 01:17:42]

Yes, making it clearer would be good!  My reading of the comment is that
CreateAuthenticator() should _internally_ make sure to call ProcessMessage()
with |first_message|, but it sounds like that's not what you mean.  In which
case, what is |first_message| used for?

[Sergey Ulanov, 2011/11/11 19:16:41]

It's used to determine what type of authenticator should be created in case the
host supports multiple authentication algorithms. For example, if
|first_message| contains <auth-token> then this method will create Authenticator
that implements our current IT2Me auth, if there is an eke message in
|first_message| then it will create EkeAuthenticator, etc.

[Wez, 2011/11/11 22:12:00]

So is the purpose of your comment to say that even though you've passed the
message in when creating the thing, you still have to call ProcessMessage()
afterwards, with the same message?

[Sergey Ulanov, 2011/11/12 00:12:49]

Yes.

+  virtual Authenticator* CreateAuthenticator(
+      const talk_base::XmlElement* first_message) = 0;
+};
+
+}  // namespace protocol
+}  // namespace remoting
+
+#endif  // REMOTING_PROTOCOL_AUTHENTICATOR_H_