Disconnect all users if too many connection requests are received for It2Me

remoting/host/it2me/it2me_host.cc

Index: remoting/host/it2me/it2me_host.cc
diff --git a/remoting/host/it2me/it2me_host.cc b/remoting/host/it2me/it2me_host.cc
index f95c7aae786491f42cc30b9c07d952675c41a0a5..1b52de5c809f4f0d43739b20d9a3801319794c9e 100644
--- a/remoting/host/it2me/it2me_host.cc
+++ b/remoting/host/it2me/it2me_host.cc
@@ -51,8 +51,25 @@ const int kMaxLoginAttempts = 5;
 
 using protocol::ValidatingAuthenticator;
 typedef ValidatingAuthenticator::Result ValidationResult;
+typedef ValidatingAuthenticator::ResultCallback ValidationResultCallback;
 typedef ValidatingAuthenticator::ValidationCallback ValidationCallback;
 
+bool GetUsernameFromJid(const std::string& remote_jid,

[Sergey Ulanov, 2017/03/15 22:25:33]

Maybe use base::Optional<std::string> for the result and remove client_username
argument?

[joedow, 2017/03/16 21:32:18]

Acknowledged.

+                        std::string* client_username) {
+  DCHECK(client_username);
+  if (!SplitJidResource(remote_jid, client_username, /*resource=*/nullptr)) {
+    LOG(ERROR) << "Malformed jid: '" << remote_jid << "'";
+    return false;
+  }
+
+  if (client_username->empty()) {
+    LOG(ERROR) << "Malformed jid, missing username: " << remote_jid;
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace
 
 It2MeHost::It2MeHost(
@@ -297,11 +314,15 @@ void It2MeHost::SetPolicyForTesting(
       done_callback);
 }
 
-ValidationCallback It2MeHost::GetValidationCallbackForTesting() {
+ValidationCallback It2MeHost::GetIncomingConnectionCallbackForTesting() {
   return base::Bind(&It2MeHost::ValidateConnectionDetails,
                     base::Unretained(this));
 }
 
+ValidationCallback It2MeHost::GetAcceptedConnectionCallbackForTesting() {
+  return base::Bind(&It2MeHost::ShowConfirmationDialog, base::Unretained(this));
+}
+
 void It2MeHost::OnPolicyUpdate(
     std::unique_ptr<base::DictionaryValue> policies) {
   // The policy watcher runs on the |ui_task_runner|.
@@ -376,7 +397,7 @@ void It2MeHost::UpdateClientDomainPolicy(const std::string& client_domain) {
 
   VLOG(2) << "UpdateClientDomainPolicy: " << client_domain;
 
-  // When setting a client  domain policy, disconnect any existing session.
+  // When setting a client domain policy, disconnect any existing session.
   if (!client_domain.empty() && IsRunning()) {
     DisconnectOnNetworkThread();
   }
@@ -469,6 +490,8 @@ void It2MeHost::OnReceivedSupportID(
       new protocol::It2MeHostAuthenticatorFactory(
           local_certificate, host_key_pair_, access_code_hash,
           base::Bind(&It2MeHost::ValidateConnectionDetails,
+                     base::Unretained(this)),
+          base::Bind(&It2MeHost::ShowConfirmationDialog,
                      base::Unretained(this))));
   host_->SetAuthenticatorFactory(std::move(factory));
 
@@ -482,24 +505,13 @@ void It2MeHost::OnReceivedSupportID(
 
 void It2MeHost::ValidateConnectionDetails(
     const std::string& remote_jid,
-    const protocol::ValidatingAuthenticator::ResultCallback& result_callback) {
+    const ValidationResultCallback& result_callback) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
   // First ensure the JID we received is valid.
   std::string client_username;
-  if (!SplitJidResource(remote_jid, &client_username, /*resource=*/nullptr)) {
-    LOG(ERROR) << "Rejecting incoming connection from " << remote_jid
-               << ": Invalid JID.";
-    result_callback.Run(
-        protocol::ValidatingAuthenticator::Result::ERROR_INVALID_ACCOUNT);
-    DisconnectOnNetworkThread();
-    return;
-  }
-
-  if (client_username.empty()) {
-    LOG(ERROR) << "Invalid user name passed in: " << remote_jid;
-    result_callback.Run(
-        protocol::ValidatingAuthenticator::Result::ERROR_INVALID_ACCOUNT);
+  if (!GetUsernameFromJid(remote_jid, &client_username)) {
+    result_callback.Run(ValidationResult::ERROR_INVALID_ACCOUNT);
     DisconnectOnNetworkThread();
     return;
   }
@@ -517,6 +529,28 @@ void It2MeHost::ValidateConnectionDetails(
     }
   }
 
+  result_callback.Run(ValidationResult::SUCCESS);
+}
+
+void It2MeHost::ShowConfirmationDialog(
+    const std::string& remote_jid,
+    const ValidationResultCallback& result_callback) {
+  std::string client_username;
+  if (!GetUsernameFromJid(remote_jid, &client_username)) {
+    result_callback.Run(ValidationResult::ERROR_INVALID_ACCOUNT);
+    DisconnectOnNetworkThread();
+    return;
+  }
+
+  // If we receive valid connection details multiple times, then we don't know
+  // which remote user (if either) is valid so disconnect everyone.
+  if (state_ != kReceivedAccessCode) {
+    LOG(ERROR) << "Received too many connection requests.";

[Sergey Ulanov, 2017/03/15 22:25:33]

Can we DCHECK here that state_ is kConnecting? I don't think it can be anything
else here.

[joedow, 2017/03/16 21:32:18]

Done.

+    result_callback.Run(ValidationResult::ERROR_TOO_MANY_CONNECTIONS);
+    DisconnectOnNetworkThread();
+    return;
+  }
+
   HOST_LOG << "Client " << client_username << " connecting.";
   SetState(kConnecting, std::string());
 
@@ -530,7 +564,7 @@ void It2MeHost::ValidateConnectionDetails(
 }
 
 void It2MeHost::OnConfirmationResult(
-    const protocol::ValidatingAuthenticator::ResultCallback& result_callback,
+    const ValidationResultCallback& result_callback,
     It2MeConfirmationDialog::Result result) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());