Multi-step authentication support in JingleSession.

BUG=105214


Review URL: http://codereview.chromium.org/8774031

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

remoting/protocol/authenticator.cc

+// 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.
+
+#include "remoting/protocol/authenticator.h"
+
+#include "remoting/base/constants.h"
+#include "third_party/libjingle/source/talk/xmllite/xmlelement.h"
+
+namespace remoting {
+namespace protocol {
+
+namespace {
+const buzz::StaticQName kAuthenticationQName = { kChromotingXmlNamespace,
+                                                 "authentication" };
+}  // namespace
+
+// static
+bool Authenticator::IsAuthenticatorMessage(const buzz::XmlElement* message) {
+  return message->Name() == kAuthenticationQName;
+}
+
+// static
+buzz::XmlElement* Authenticator::CreateEmptyAuthenticatorMessage() {
+  return new buzz::XmlElement(kAuthenticationQName);
+}
+
+// static
+const buzz::XmlElement* Authenticator::FindAuthenticatorMessage(
+    const buzz::XmlElement* message) {
+  return message->FirstNamed(kAuthenticationQName);
+}
+
+}  // namespace protocol
+}  // namespace remoting

remoting/protocol/authenticator.h

@@ -53,6 +53,17 @@ class Authenticator {
     REJECTED,
   };
 
+  // Returns true if |message| is an Authenticator message.
+  static bool IsAuthenticatorMessage(const buzz::XmlElement* message);
+
+  // Creates an empty Authenticator message, owned by the caller.
+  static buzz::XmlElement* CreateEmptyAuthenticatorMessage();
+
+  // Finds Authenticator message among child elements of |message|, or
+  // returns NULL otherwise.
+  static const buzz::XmlElement* FindAuthenticatorMessage(
+      const buzz::XmlElement* message);
+
   Authenticator() {}
   virtual ~Authenticator() {}
 

remoting/protocol/content_description.cc

@@ -7,6 +7,7 @@
 #include "base/logging.h"
 #include "base/string_number_conversions.h"
 #include "remoting/base/constants.h"
+#include "remoting/protocol/authenticator.h"
 #include "third_party/libjingle/source/talk/xmllite/xmlelement.h"
 
 using buzz::QName;
@@ -27,7 +28,6 @@ const char kControlTag[] = "control";
 const char kEventTag[] = "event";
 const char kVideoTag[] = "video";
 const char kResolutionTag[] = "initial-resolution";
-const char kAuthenticationTag[] = "authentication";
 
 const char kTransportAttr[] = "transport";
 const char kVersionAttr[] = "version";
@@ -198,8 +198,7 @@ XmlElement* ContentDescription::ToXml() const {
   root->AddElement(resolution_tag);
 
   if (authenticator_message_.get()) {
-    DCHECK(authenticator_message_->Name() ==
-           QName(kChromotingXmlNamespace, kAuthenticationTag));
+    DCHECK(Authenticator::IsAuthenticatorMessage(authenticator_message_.get()));
     root->AddElement(new XmlElement(*authenticator_message_));
   }
 
@@ -266,8 +265,7 @@ ContentDescription* ContentDescription::ParseXml(
     *config->mutable_initial_resolution() = resolution;
 
     scoped_ptr<XmlElement> authenticator_message;
-    child = element->FirstNamed(QName(kChromotingXmlNamespace,
-                                      kAuthenticationTag));
+    child = Authenticator::FindAuthenticatorMessage(element);
     if (child)
       authenticator_message.reset(new XmlElement(*child));
 

remoting/protocol/jingle_session.cc

@@ -45,6 +45,8 @@ JingleSession::JingleSession(
   jid_ = cricket_session_->remote_name();
   cricket_session_->SignalState.connect(this, &JingleSession::OnSessionState);
   cricket_session_->SignalError.connect(this, &JingleSession::OnSessionError);
+  cricket_session_->SignalInfoMessage.connect(
+      this, &JingleSession::OnSessionInfoMessage);
   cricket_session_->SignalReceivedTerminateReason.connect(
       this, &JingleSession::OnTerminateReason);
 }
@@ -246,6 +248,25 @@ void JingleSession::OnSessionError(
   }
 }
 
+void JingleSession::OnSessionInfoMessage(cricket::Session* session,
+                                         const buzz::XmlElement* message) {
+  DCHECK_EQ(cricket_session_,session);
+
+  const buzz::XmlElement* auth_message =
+      Authenticator::FindAuthenticatorMessage(message);
+  if (auth_message) {
+    if (state_ != CONNECTED ||
+        authenticator_->state() != Authenticator::WAITING_MESSAGE) {
+      LOG(WARNING) << "Received unexpected authenticator message "
+                   << auth_message->Str();
+      return;
+    }
+
+    authenticator_->ProcessMessage(auth_message);
+    ProcessAuthenticationStep();
+  }
+}
+
 void JingleSession::OnTerminateReason(cricket::Session* session,
                                       const std::string& reason) {
   terminate_reason_ = reason;
@@ -294,16 +315,10 @@ bool JingleSession::InitializeConfigFromDescription(
     return false;
   }
 
-  DCHECK(authenticator_->state() == Authenticator::WAITING_MESSAGE);
+  DCHECK_EQ(authenticator_->state(), Authenticator::WAITING_MESSAGE);
   authenticator_->ProcessMessage(auth_message);
-  // Support for more than two auth message is not implemented yet.
-  DCHECK(authenticator_->state() != Authenticator::WAITING_MESSAGE &&
-         authenticator_->state() != Authenticator::MESSAGE_READY);
-
-  if (authenticator_->state() != Authenticator::ACCEPTED) {
-    return false;
-  }
 
+  // Initialize session configuration.
   SessionConfig config;
   if (!content_description->config()->GetFinalConfig(&config)) {
     LOG(ERROR) << "Connection response does not specify configuration";
@@ -333,8 +348,12 @@ void JingleSession::OnAccept() {
 
   SetState(CONNECTED);
 
-  if (authenticator_->state() == Authenticator::ACCEPTED)
+  // Process authentication.
+  if (authenticator_->state() == Authenticator::ACCEPTED) {
     SetState(AUTHENTICATED);
+  } else {
+    ProcessAuthenticationStep();
+  }
 }
 
 void JingleSession::OnTerminate() {
@@ -398,8 +417,6 @@ void JingleSession::AcceptConnection() {
 
   DCHECK(authenticator_->state() == Authenticator::WAITING_MESSAGE);
   authenticator_->ProcessMessage(auth_message);
-  // Support for more than two auth message is not implemented yet.
-  DCHECK(authenticator_->state() != Authenticator::WAITING_MESSAGE);
   if (authenticator_->state() == Authenticator::REJECTED) {
     CloseInternal(net::ERR_CONNECTION_FAILED, AUTHENTICATION_FAILED);
     return;
@@ -412,11 +429,29 @@ void JingleSession::AcceptConnection() {
   buzz::XmlElement* auth_reply = NULL;
   if (authenticator_->state() == Authenticator::MESSAGE_READY)
     auth_reply = authenticator_->GetNextMessage();
-  DCHECK_EQ(authenticator_->state(), Authenticator::ACCEPTED);
+  DCHECK_NE(authenticator_->state(), Authenticator::MESSAGE_READY);
   cricket_session_->Accept(
       CreateSessionDescription(candidate_config, auth_reply));
 }
 
+void JingleSession::ProcessAuthenticationStep() {
+  DCHECK_EQ(state_, CONNECTED);
+
+  if (authenticator_->state() == Authenticator::MESSAGE_READY) {
+    buzz::XmlElement* auth_message = authenticator_->GetNextMessage();
+    cricket::XmlElements message;
+    message.push_back(auth_message);
+    cricket_session_->SendInfoMessage(message);
+  }
+  DCHECK_NE(authenticator_->state(), Authenticator::MESSAGE_READY);
+
+  if (authenticator_->state() == Authenticator::ACCEPTED) {
+    SetState(AUTHENTICATED);
+  } else if (authenticator_->state() == Authenticator::REJECTED) {
+    CloseInternal(net::ERR_CONNECTION_ABORTED, AUTHENTICATION_FAILED);
+  }
+}
+
 void JingleSession::AddChannelConnector(
     const std::string& name, JingleChannelConnector* connector) {
   DCHECK(channel_connectors_.find(name) == channel_connectors_.end());

remoting/protocol/jingle_session.h

@@ -74,14 +74,15 @@ class JingleSession : public protocol::Session,
   bool InitializeConfigFromDescription(
       const cricket::SessionDescription* description);
 
-  // Used for Session.SignalState sigslot.
+  // Handlers for |cricket_session_| signals.
   void OnSessionState(cricket::BaseSession* session,
                       cricket::BaseSession::State state);
-  // Used for Session.SignalError sigslot.
   void OnSessionError(cricket::BaseSession* session,
                       cricket::BaseSession::Error error);
-  // Used for Session.SignalReceivedTerminateReason sigslot.
-  void OnTerminateReason(cricket::Session* session, const std::string& reason);
+  void OnSessionInfoMessage(cricket::Session* session,
+                            const buzz::XmlElement* message);
+  void OnTerminateReason(cricket::Session* session,
+                         const std::string& reason);
 
   void OnInitiate();
   void OnAccept();
@@ -91,6 +92,8 @@ class JingleSession : public protocol::Session,
   // accepts/rejects connection.
   void AcceptConnection();
 
+  void ProcessAuthenticationStep();
+
   void AddChannelConnector(const std::string& name,
                            JingleChannelConnector* connector);
 

remoting/protocol/jingle_session_unittest.cc

@@ -171,14 +171,14 @@ class JingleSessionTest : public testing::Test {
     {
       InSequence dummy;
 
+      EXPECT_CALL(host_connection_callback_,
+                  OnStateChange(Session::CONNECTED))
+          .Times(AtMost(1));
       if (expect_fail) {
         EXPECT_CALL(host_connection_callback_,
                     OnStateChange(Session::FAILED))
             .Times(1);
       } else {
-        EXPECT_CALL(host_connection_callback_,
-                    OnStateChange(Session::CONNECTED))
-            .Times(1);
         EXPECT_CALL(host_connection_callback_,
                     OnStateChange(Session::AUTHENTICATED))
             .Times(1);
@@ -191,14 +191,14 @@ class JingleSessionTest : public testing::Test {
       EXPECT_CALL(client_connection_callback_,
                   OnStateChange(Session::CONNECTING))
           .Times(1);
+      EXPECT_CALL(client_connection_callback_,
+                  OnStateChange(Session::CONNECTED))
+          .Times(AtMost(1));
       if (expect_fail) {
         EXPECT_CALL(client_connection_callback_,
                     OnStateChange(Session::FAILED))
             .Times(1);
       } else {
-        EXPECT_CALL(client_connection_callback_,
-                    OnStateChange(Session::CONNECTED))
-            .Times(1);
         EXPECT_CALL(client_connection_callback_,
                     OnStateChange(Session::AUTHENTICATED))
             .Times(1);
@@ -326,12 +326,24 @@ TEST_F(JingleSessionTest, Connect) {
   InitiateConnection(1, FakeAuthenticator::ACCEPT, false);
 }
 
-// Verify that we can't connect two endpoints with mismatched secrets.
+// Verify that we can connect two endpoints with multi-step authentication.
+TEST_F(JingleSessionTest, ConnectMultistep) {
+  CreateServerPair(3, FakeAuthenticator::ACCEPT);
+  InitiateConnection(3, FakeAuthenticator::ACCEPT, false);
+}
+
+// Verify that connection is terminated when auth fails.
 TEST_F(JingleSessionTest, ConnectBadAuth) {
   CreateServerPair(1, FakeAuthenticator::REJECT);
   InitiateConnection(1, FakeAuthenticator::ACCEPT, true);
 }
 
+// Verify that connection is terminted when multi-step auth fails.
+TEST_F(JingleSessionTest, ConnectBadMultistepAuth) {
+  CreateServerPair(3, FakeAuthenticator::REJECT);
+  InitiateConnection(3, FakeAuthenticator::ACCEPT, true);
+}
+
 TEST_F(JingleSessionTest, ConnectBadChannelAuth) {
   CreateServerPair(1, FakeAuthenticator::REJECT_CHANNEL);
   ASSERT_NO_FATAL_FAILURE(
@@ -374,6 +386,21 @@ TEST_F(JingleSessionTest, TestTcpChannel) {
   tester.CheckResults();
 }
 
+// Verify that we can connect channels with multistep auth.
+TEST_F(JingleSessionTest, TestMultistepAuthTcpChannel) {
+  CreateServerPair(3, FakeAuthenticator::ACCEPT);
+  ASSERT_NO_FATAL_FAILURE(
+      InitiateConnection(3, FakeAuthenticator::ACCEPT, false));
+
+  ASSERT_NO_FATAL_FAILURE(CreateChannel());
+
+  StreamConnectionTester tester(host_socket_.get(), client_socket_.get(),
+                                kMessageSize, kMessages);
+  tester.Start();
+  message_loop_.Run();
+  tester.CheckResults();
+}
+
 // Verify that data can be transmitted over the video RTP channel.
 TEST_F(JingleSessionTest, TestUdpChannel) {
   CreateServerPair(1, FakeAuthenticator::ACCEPT);

remoting/protocol/v1_authenticator.cc

@@ -20,7 +20,6 @@ namespace remoting {
 namespace protocol {
 
 namespace {
-const char kAuthenticationTag[] = "authentication";
 const char kAuthTokenTag[] = "auth-token";
 const char kCertificateTag[] = "certificate";
 }  // namespace
@@ -64,19 +63,16 @@ void V1ClientAuthenticator::ProcessMessage(const XmlElement* message) {
 XmlElement* V1ClientAuthenticator::GetNextMessage() {
   DCHECK_EQ(state_, MESSAGE_READY);
 
-  XmlElement* authentication_tag = new XmlElement(
-      QName(kChromotingXmlNamespace, kAuthenticationTag));
-
+  XmlElement* message = CreateEmptyAuthenticatorMessage();
   std::string token =
       protocol::GenerateSupportAuthToken(local_jid_, shared_secret_);
-
   XmlElement* auth_token_tag = new XmlElement(
       QName(kChromotingXmlNamespace, kAuthTokenTag));
   auth_token_tag->SetBodyText(token);
-  authentication_tag->AddElement(auth_token_tag);
+  message->AddElement(auth_token_tag);
 
   state_ = WAITING_MESSAGE;
-  return authentication_tag;
+  return message;
 }
 
 ChannelAuthenticator*
@@ -121,9 +117,7 @@ void V1HostAuthenticator::ProcessMessage(const XmlElement* message) {
 XmlElement* V1HostAuthenticator::GetNextMessage() {
   DCHECK_EQ(state_, MESSAGE_READY);
 
-  XmlElement* message = new XmlElement(
-      QName(kChromotingXmlNamespace, kAuthenticationTag));
-
+  XmlElement* message = CreateEmptyAuthenticatorMessage();
   buzz::XmlElement* certificate_tag = new XmlElement(
       buzz::QName(kChromotingXmlNamespace, kCertificateTag));
   std::string base64_cert;

remoting/remoting.gyp

@@ -743,6 +743,7 @@
       'sources': [
         'protocol/auth_util.cc',
         'protocol/auth_util.h',
+        'protocol/authenticator.cc',
         'protocol/authenticator.h',
         'protocol/buffered_socket_writer.cc',
         'protocol/buffered_socket_writer.h',