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

remoting/host/it2me/it2me_host.cc

 // 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.
 
 #include "remoting/host/it2me/it2me_host.h"
 
 #include <cstdint>
 #include <memory>
 #include <string>
 #include <utility>
@@ skipping 358 matching lines @@
   }
 
   required_host_domain_ = host_domain;
 }
 
 void It2MeHost::UpdateClientDomainPolicy(const std::string& client_domain) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
   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();
   }
 
   required_client_domain_ = client_domain;
 }
 
 void It2MeHost::SetState(It2MeHostState state,
                          const std::string& error_message) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
@@ skipping 93 matching lines @@
 void It2MeHost::ValidateConnectionDetails(
     const std::string& remote_jid,
     const protocol::ValidatingAuthenticator::ResultCallback& 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);
+    result_callback.Run(ValidationResult::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);
+    result_callback.Run(ValidationResult::ERROR_INVALID_ACCOUNT);
     DisconnectOnNetworkThread();
     return;
   }
 
   // Check the client domain policy.
   if (!required_client_domain_.empty()) {
     if (!base::EndsWith(client_username,
                         std::string("@") + required_client_domain_,
                         base::CompareCase::INSENSITIVE_ASCII)) {
       LOG(ERROR) << "Rejecting incoming connection from " << remote_jid
                  << ": Domain mismatch.";
       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) {

[Sergey Ulanov, 2017/03/02 23:07:29]

I don't think we want to shutdown the host when the user mistypes access code.
Currently the host keeps running and client can correct access code and try
connecting again. I'd like to keep this behavior.

Currently ValidatingAuthenticator calls validation callback before processing
the first message. Can it be updated to perform validation after base
authentication is complete? I think it should be enough to fix this bug.

[joedow, 2017/03/13 23:26:57]

I missed that scenario so thanks for pointing that out.  I've broken validation
into two steps, one which is run on incoming connections to weed out obviously
wrong ones before doing additional validation and a second step which occurs
after the lower level authenticators have all succeeded.  This second step is
the right place for the confirmation dialog as the JID has been validated, a
connection method has been negotiated, and we know the access code is valid.

+    LOG(ERROR) << "Received too many connection requests.";
+    result_callback.Run(ValidationResult::ERROR_TOO_MANY_CONNECTIONS);
+    DisconnectOnNetworkThread();
+    return;
+  }
+
   HOST_LOG << "Client " << client_username << " connecting.";
   SetState(kConnecting, std::string());
 
   // Show a confirmation dialog to the user to allow them to confirm/reject it.
   confirmation_dialog_proxy_.reset(new It2MeConfirmationDialogProxy(
       host_context_->ui_task_runner(), std::move(confirmation_dialog_)));
 
   confirmation_dialog_proxy_->Show(
       client_username, base::Bind(&It2MeHost::OnConfirmationResult,
                                   base::Unretained(this), result_callback));
@@ skipping 30 matching lines @@
   DCHECK(context->ui_task_runner()->BelongsToCurrentThread());
 
   std::unique_ptr<PolicyWatcher> policy_watcher =
       PolicyWatcher::Create(policy_service, context->file_task_runner());
   return new It2MeHost(std::move(context), std::move(policy_watcher),
                        It2MeConfirmationDialog::Create(), observer,
                        std::move(signal_strategy), username, directory_bot_jid);
 }
 
 }  // namespace remoting