Host port range policy is no longer ignored in it2me host

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 173 matching lines @@
 
   // Request registration of the host for support.
   std::unique_ptr<RegisterSupportHostRequest> register_request(
       new RegisterSupportHostRequest(
           signal_strategy_.get(), host_key_pair_, directory_bot_jid_,
           base::Bind(&It2MeHost::OnReceivedSupportID, base::Unretained(this))));
 
   // Beyond this point nothing can fail, so save the config and request.
   register_request_ = std::move(register_request);
 
-  // If NAT traversal is off then limit port range to allow firewall pin-holing.
-  HOST_LOG << "NAT state: " << nat_traversal_enabled_;
   protocol::NetworkSettings network_settings(
      nat_traversal_enabled_ ?
      protocol::NetworkSettings::NAT_TRAVERSAL_FULL :
      protocol::NetworkSettings::NAT_TRAVERSAL_DISABLED);
+  network_settings.port_range = udp_port_range_;
+
+  // If NAT traversal is off then limit port range to allow firewall pin-holing.
+  HOST_LOG << "NAT state: " << nat_traversal_enabled_;
   if (!nat_traversal_enabled_) {
     network_settings.port_range.min_port =

[Sergey Ulanov, 2017/05/25 22:24:38]

Setting port range here makes sense only if it isn't already set by the policy.
I.e.
  if (!udp_port_range_.is_null()) {
    network_settings.port_range = udp_port_range_;
  } else if (!nat_traversal_enabled_) {
    network_settings.port_range.min_port =
      protocol::NetworkSettings::kDefaultMinPort;
    network_settings.port_range.max_port =
      protocol::NetworkSettings::kDefaultMaxPort;    
  }

[Gus Smith, 2017/05/25 22:55:58]

Done. I don't think I fully understand what should take precedence over what,
though - I assumed that NAT traversal being disabled should force the port range
to the default port range to allow for firewall pin-holing as the comment
states. Is this not the case?

[Sergey Ulanov, 2017/05/26 00:40:08]

If an admin disables NAT traversal then they also have control of the port range
policy. They can set the policy to the default range if that's what they want.
This code was added long time ago when we didn't have PortRange policy. Now that
we have it we should respect it regardless of the NAT traversal state. This
behavior is consistent with the Me2Me host, see
https://codesearch.chromium.org/chromium/src/remoting/host/remoting_me2me_hos...

         protocol::NetworkSettings::kDefaultMinPort;
     network_settings.port_range.max_port =
         protocol::NetworkSettings::kDefaultMaxPort;
   }
 
   scoped_refptr<protocol::TransportContext> transport_context =
       new protocol::TransportContext(
           signal_strategy_.get(),
           base::WrapUnique(new protocol::ChromiumPortAllocatorFactory()),
           base::WrapUnique(new ChromiumUrlRequestFactory(
@@ skipping 111 matching lines @@
   const base::ListValue* client_domain_list;
   if (policies->GetList(policy::key::kRemoteAccessHostClientDomainList,
                         &client_domain_list)) {
     std::vector<std::string> client_domain_list_vector;
     for (const auto& value : *client_domain_list) {
       client_domain_list_vector.push_back(value.GetString());
     }
     UpdateClientDomainListPolicy(std::move(client_domain_list_vector));
   }
 
+  std::string port_range_string;
+  if (policies->GetString(policy::key::kRemoteAccessHostUdpPortRange,
+                          &port_range_string)) {
+    UpdateHostUdpPortRangePolicy(port_range_string);
+  }
+
   policy_received_ = true;
 
   if (!pending_connect_.is_null()) {
     base::ResetAndReturn(&pending_connect_).Run();
   }
 }
 
 void It2MeHost::UpdateNatPolicy(bool nat_traversal_enabled) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
@@ skipping 36 matching lines @@
           << base::JoinString(client_domain_list, ", ");
 
   // When setting a client  domain policy, disconnect any existing session.
   if (!client_domain_list.empty() && IsRunning()) {
     DisconnectOnNetworkThread();
   }
 
   required_client_domain_list_ = std::move(client_domain_list);
 }
 
+void It2MeHost::UpdateHostUdpPortRangePolicy(
+    const std::string& port_range_string) {
+  DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
+
+  VLOG(2) << "UpdateHostUdpPortRangePolicy: " << port_range_string;
+
+  if (IsRunning()) {
+    DisconnectOnNetworkThread();
+  }
+
+  if (!PortRange::Parse(port_range_string, &udp_port_range_)) {
+    // PolicyWatcher verifies that the value is formatted correctly.
+    LOG(FATAL) << "Invalid port range: " << port_range_string;
+  }
+}
+
 void It2MeHost::SetState(It2MeHostState state,
                          const std::string& error_message) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
   switch (state_) {
     case kDisconnected:
       DCHECK(state == kStarting ||
              state == kError) << state;
       break;
     case kStarting:
@@ skipping 177 matching lines @@
     std::unique_ptr<SignalStrategy> signal_strategy,
     const std::string& username,
     const std::string& directory_bot_jid) {
   DCHECK(context->ui_task_runner()->BelongsToCurrentThread());
   return new It2MeHost(
       std::move(context), base::MakeUnique<It2MeConfirmationDialogFactory>(),
       observer, std::move(signal_strategy), username, directory_bot_jid);
 }
 
 }  // namespace remoting