Log an error and fail to connect on invalid Host response, rather than crashing.

BUG=80583
TEST=Modify a Chromoting Host to supply an malformed certificate, and connect to it.  Client should fail to connect, and log an error to the console, rather than crashing.


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

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

3 files changed, 74 insertions, 39 deletions

diff --git a/remoting/protocol/jingle_session.cc b/remoting/protocol/jingle_session.cc
index d6fd990..a53aeb4 100644
--- a/remoting/protocol/jingle_session.cc
+++ b/remoting/protocol/jingle_session.cc
@@ -405,49 +405,71 @@ bool JingleSession::EstablishSSLConnection(
   return true;
 }
 
-void JingleSession::OnAccept() {
-  // TODO(hclam): Need to close the adapters on failuire otherwise it will
-  // crash in the destructor.
+bool JingleSession::InitializeConfigFromDescription(
+    const cricket::SessionDescription* description) {
+  // We should only be called after ParseContent has succeeded, in which
+  // case there will always be a Chromoting session configuration.
+  const cricket::ContentInfo* content =
+    description->FirstContentByType(kChromotingXmlNamespace);
+  CHECK(content);
+  const protocol::ContentDescription* content_description =
+    static_cast<const protocol::ContentDescription*>(content->description);
+  CHECK(content_description);
+
+  server_cert_ = content_description->certificate();
+  if (!server_cert_) {
+    LOG(ERROR) << "Connection response does not specify certificate";
+    return false;
+  }
 
-  // Set the config if we are the one who initiated the session.
-  if (cricket_session_->initiator()) {
-    const cricket::ContentInfo* content =
-        cricket_session_->remote_description()->FirstContentByType(
-            kChromotingXmlNamespace);
-    CHECK(content);
+  scoped_ptr<SessionConfig> config(
+    content_description->config()->GetFinalConfig());
+  if (!config.get()) {
+    LOG(ERROR) << "Connection response does not specify configuration";
+    return false;
+  }
+  if (!candidate_config()->IsSupported(config.get())) {
+    LOG(ERROR) << "Connection response specifies an invalid configuration";
+    return false;
+  }
 
-    const protocol::ContentDescription* content_description =
-        static_cast<const protocol::ContentDescription*>(content->description);
-    server_cert_ = content_description->certificate();
-    CHECK(server_cert_);
+  set_config(config.release());
+  return true;
+}
 
-    SessionConfig* config = content_description->config()->GetFinalConfig();
+bool JingleSession::InitializeChannels() {
+  if (!EstablishSSLConnection(control_channel_.release(),
+                                    &control_ssl_socket_)) {
+    LOG(ERROR) << "Establish control channel failed";
+    return false;
+  }
+  if (!EstablishSSLConnection(event_channel_.release(),
+                                    &event_ssl_socket_)) {
+    LOG(ERROR) << "Establish event channel failed";
+    return false;
+  }
+  if (!EstablishSSLConnection(video_channel_.release(),
+                                    &video_ssl_socket_)) {
+    LOG(ERROR) << "Establish video channel failed";
+    return false;
+  }
+  return true;
+}
 
-    // Terminate the session if the config we received is invalid.
-    if (!config || !candidate_config()->IsSupported(config)) {
-      // TODO(sergeyu): Inform the user that the host is misbehaving?
-      LOG(ERROR) << "Terminating outgoing session after an "
-          "invalid session description has been received.";
-      cricket_session_->Terminate();
+void JingleSession::OnAccept() {
+  // If we initiated the session, store the candidate configuration that the
+  // host responded with, to refer to later.
+  if (cricket_session_->initiator()) {
+    if (!InitializeConfigFromDescription(
+        cricket_session_->remote_description())) {
+      CloseInternal(net::ERR_CONNECTION_FAILED, true);
       return;
     }
-    set_config(config);
   }
 
-  bool ret = EstablishSSLConnection(control_channel_.release(),
-                                    &control_ssl_socket_);
-  if (ret) {
-    ret = EstablishSSLConnection(event_channel_.release(),
-                                 &event_ssl_socket_);
-  }
-  if (ret) {
-    ret = EstablishSSLConnection(video_channel_.release(),
-                                 &video_ssl_socket_);
-  }
-
-  if (!ret) {
-    LOG(ERROR) << "Failed to establish SSL connections";
-    cricket_session_->Terminate();
+  if (!InitializeChannels()) {
+    CloseInternal(net::ERR_CONNECTION_FAILED, true);
+    return;
   }
 }
 
diff --git a/remoting/protocol/jingle_session.h b/remoting/protocol/jingle_session.h
index b47a7aa..d919d56 100644
--- a/remoting/protocol/jingle_session.h
+++ b/remoting/protocol/jingle_session.h
@@ -97,6 +97,15 @@ class JingleSession : public protocol::Session,
   bool HasSession(cricket::Session* cricket_session);
   cricket::Session* ReleaseSession();
 
+  // Initialize the session configuration from a received connection response
+  // stanza.
+  bool InitializeConfigFromDescription(
+    const cricket::SessionDescription* description);
+
+  // Initialize PseudoTCP + SSL on each of the video, control and input
+  // channels.  The channels must have been created before this is called.
+  bool InitializeChannels();
+
   // Helper method to create and initialize PseudoTCP + SSL socket on
   // top of the provided |channel|. The resultant SSL socket is
   // written to |ssl_socket|. Return true if successful.
diff --git a/remoting/protocol/jingle_session_manager.cc b/remoting/protocol/jingle_session_manager.cc
index 29eacd7..9a1827b 100644
--- a/remoting/protocol/jingle_session_manager.cc
+++ b/remoting/protocol/jingle_session_manager.cc
@@ -436,20 +436,24 @@ bool JingleSessionManager::ParseContent(
     scoped_refptr<net::X509Certificate> certificate;
     child = element->FirstNamed(QName(kChromotingXmlNamespace,
                                       kAuthenticationTag));
-    if (child)
+    if (child) {
       child = child->FirstNamed(QName(kChromotingXmlNamespace,
                                       kCertificateTag));
-
+    }
     if (child) {
       std::string base64_cert = child->BodyText();
       std::string der_cert;
-      bool ret = base::Base64Decode(base64_cert, &der_cert);
-      if (!ret) {
+      if (!base::Base64Decode(base64_cert, &der_cert)) {
         LOG(ERROR) << "Failed to decode certificate received from the peer.";
         return false;
       }
+
       certificate = net::X509Certificate::CreateFromBytes(der_cert.data(),
                                                           der_cert.length());
+      if (!certificate) {
+        LOG(ERROR) << "Failed to create platform-specific certificate handle";
+        return false;
+      }
     }
 
     *content = new ContentDescription(config.release(), auth_token,