Malformed PortRange or ThirdPartyAuthConfig should trigger OnPolicyError.

Before this change a malformed value of RemoteAccessHostUdpPortRange policy
(i.e. "123456-blah") was ignored and a default value was used.  Similarily for
the 3 third-party-auth-config policies (RemoteAccessHostTokenUrl,
...TokenValidationUrl and ...TokenValidationCertificateIssuer) we were falling
back to a secure default, but not reporting a policy error.  After this change
such malformed values will trigger an OnPolicyError callback.

Notes:
- Guaranteeing that PolicyWatcher always returns valid policy values, removes
  the need for a "rejecting" Me2MeHostAuthenticatorFactory.
- Moving PortRange and ThirdPartyAuthConfig to separate compilation units
  helps readability elsewhere + encourages better unit tests coverage.
- Initially I tried to wrap all policies in a new ChromotingPolicies class,
  but eventually went back to prevalidating and passing base::DictionaryValue.
  - Arguments for using ChromotingPolicies:
    - Helps avoid overtesting in policy_watcher_unittests.cc (i.e. helps focus
      the tests on a single policy value).
  - Arguments for using base::DictionaryValue:
    - Minimizes changes.
    - Keeps things simple (as opposed to having to introduce a custom
      equivalent of optional<T> [nothing similar present in Chromium AFAICT]).
  - Neutral:
    - Strong-typing of ChromotingPolicies didn't help readability as much as I
      expected and hoped for.

BUG=427513
TEST=remoting_unittests

Review URL: https://codereview.chromium.org/966433002

Cr-Commit-Position: refs/heads/master@{#318910}

remoting/host/it2me/it2me_host.cc

@@ -229,8 +229,10 @@ void It2MeHost::FinishConnect() {
      protocol::NetworkSettings::NAT_TRAVERSAL_FULL :
      protocol::NetworkSettings::NAT_TRAVERSAL_DISABLED);
   if (!nat_traversal_enabled_) {
-    network_settings.min_port = protocol::NetworkSettings::kDefaultMinPort;
-    network_settings.max_port = protocol::NetworkSettings::kDefaultMaxPort;
+    network_settings.port_range.min_port =
+        protocol::NetworkSettings::kDefaultMinPort;
+    network_settings.port_range.max_port =
+        protocol::NetworkSettings::kDefaultMaxPort;
   }
 
   // Create the host.

remoting/host/policy_watcher.cc

@@ -21,6 +21,8 @@
 #include "components/policy/core/common/schema_registry.h"
 #include "policy/policy_constants.h"
 #include "remoting/host/dns_blackhole_checker.h"
+#include "remoting/host/third_party_auth_config.h"
+#include "remoting/protocol/port_range.h"
 
 #if !defined(NDEBUG)
 #include "base/json/json_reader.h"
@@ -44,15 +46,15 @@ namespace {
 // Copies all policy values from one dictionary to another, using values from
 // |default_values| if they are not set in |from|.
 scoped_ptr<base::DictionaryValue> CopyValuesAndAddDefaults(
-    const base::DictionaryValue* from,
-    const base::DictionaryValue* default_values) {
-  scoped_ptr<base::DictionaryValue> to(default_values->DeepCopy());
-  for (base::DictionaryValue::Iterator i(*default_values); !i.IsAtEnd();
+    const base::DictionaryValue& from,
+    const base::DictionaryValue& default_values) {
+  scoped_ptr<base::DictionaryValue> to(default_values.DeepCopy());
+  for (base::DictionaryValue::Iterator i(default_values); !i.IsAtEnd();
        i.Advance()) {
     const base::Value* value = nullptr;
 
     // If the policy isn't in |from|, use the default.
-    if (!from->Get(i.key(), &value)) {
+    if (!from.Get(i.key(), &value)) {
       continue;
     }
 
@@ -63,7 +65,7 @@ scoped_ptr<base::DictionaryValue> CopyValuesAndAddDefaults(
 #if !defined(NDEBUG)
   // Replace values with those specified in DebugOverridePolicies, if present.
   std::string policy_overrides;
-  if (from->GetString(key::kRemoteAccessHostDebugOverridePolicies,
+  if (from.GetString(key::kRemoteAccessHostDebugOverridePolicies,
                      &policy_overrides)) {
     scoped_ptr<base::Value> value(base::JSONReader::Read(policy_overrides));
     const base::DictionaryValue* override_values;
@@ -92,6 +94,56 @@ scoped_ptr<policy::SchemaRegistry> CreateSchemaRegistry() {
   return schema_registry.Pass();
 }
 
+scoped_ptr<base::DictionaryValue> CopyChromotingPoliciesIntoDictionary(
+    const policy::PolicyMap& current) {
+  const char kPolicyNameSubstring[] = "RemoteAccessHost";
+  scoped_ptr<base::DictionaryValue> policy_dict(new base::DictionaryValue());
+  for (auto it = current.begin(); it != current.end(); ++it) {
+    const std::string& key = it->first;
+    const base::Value* value = it->second.value;
+
+    // Copying only Chromoting-specific policies helps avoid false alarms
+    // raised by NormalizePolicies below (such alarms shutdown the host).
+    // TODO(lukasza): Removing this somewhat brittle filtering will be possible
+    //                after having separate, Chromoting-specific schema.
+    if (key.find(kPolicyNameSubstring) != std::string::npos) {
+      policy_dict->Set(key, value->DeepCopy());
+    }
+  }
+
+  return policy_dict.Pass();
+}
+
+// Takes a dictionary containing only 1) recognized policy names and 2)
+// well-typed policy values and further verifies policy contents.
+bool VerifyWellformedness(const base::DictionaryValue& changed_policies) {
+  // Verify ThirdPartyAuthConfig policy.
+  ThirdPartyAuthConfig not_used;
+  switch (ThirdPartyAuthConfig::Parse(changed_policies, &not_used)) {
+    case ThirdPartyAuthConfig::NoPolicy:
+    case ThirdPartyAuthConfig::ParsingSuccess:
+      break;  // Well-formed.
+    case ThirdPartyAuthConfig::InvalidPolicy:
+      return false;  // Malformed.
+    default:
+      NOTREACHED();
+      return false;
+  }
+
+  // Verify UdpPortRange policy.
+  std::string udp_port_range_string;
+  PortRange udp_port_range;
+  if (changed_policies.GetString(policy::key::kRemoteAccessHostUdpPortRange,
+                                 &udp_port_range_string)) {
+    if (!PortRange::Parse(udp_port_range_string, &udp_port_range)) {
+      return false;
+    }
+  }
+
+  // Report that all the policies were well-formed.
+  return true;
+}
+
 }  // namespace
 
 void PolicyWatcher::StartWatching(
@@ -114,36 +166,6 @@ void PolicyWatcher::StartWatching(
   }
 }
 
-void PolicyWatcher::UpdatePolicies(
-    const base::DictionaryValue* new_policies_raw) {
-  DCHECK(CalledOnValidThread());
-
-  // Use default values for any missing policies.
-  scoped_ptr<base::DictionaryValue> new_policies =
-      CopyValuesAndAddDefaults(new_policies_raw, default_values_.get());
-
-  // Find the changed policies.
-  scoped_ptr<base::DictionaryValue> changed_policies(
-      new base::DictionaryValue());
-  base::DictionaryValue::Iterator iter(*new_policies);
-  while (!iter.IsAtEnd()) {
-    base::Value* old_policy;
-    if (!(old_policies_->Get(iter.key(), &old_policy) &&
-          old_policy->Equals(&iter.value()))) {
-      changed_policies->Set(iter.key(), iter.value().DeepCopy());
-    }
-    iter.Advance();
-  }
-
-  // Save the new policies.
-  old_policies_.swap(new_policies);
-
-  // Notify our client of the changed policies.
-  if (!changed_policies->empty()) {
-    policy_updated_callback_.Run(changed_policies.Pass());
-  }
-}
-
 void PolicyWatcher::SignalPolicyError() {
   old_policies_->Clear();
   policy_error_callback_.Run();
@@ -201,24 +223,7 @@ const policy::Schema* PolicyWatcher::GetPolicySchema() const {
   return owned_schema_registry_->schema_map()->GetSchema(GetPolicyNamespace());
 }
 
-void PolicyWatcher::OnPolicyUpdated(const policy::PolicyNamespace& ns,
-                                    const policy::PolicyMap& previous,
-                                    const policy::PolicyMap& current) {
-  const char kPolicyNamePrefix[] = "RemoteAccessHost";
-  scoped_ptr<base::DictionaryValue> policy_dict(new base::DictionaryValue());
-  for (auto it = current.begin(); it != current.end(); ++it) {
-    const std::string& key = it->first;
-    const base::Value* value = it->second.value;
-
-    // Copying only Chromoting-specific policies helps avoid false alarms
-    // raised by Schema::Normalize below (such alarms shutdown the host).
-    // TODO(lukasza): Removing this somewhat brittle filtering will be possible
-    //                after having separate, Chromoting-specific schema.
-    if (key.find(kPolicyNamePrefix) != std::string::npos) {
-      policy_dict->Set(key, value->DeepCopy());
-    }
-  }
-
+bool PolicyWatcher::NormalizePolicies(base::DictionaryValue* policy_dict) {
   // Allowing unrecognized policy names allows presence of
   // 1) comments (i.e. JSON of the form: { "_comment": "blah", ... }),
   // 2) policies intended for future/newer versions of the host,
@@ -231,16 +236,95 @@ void PolicyWatcher::OnPolicyUpdated(const policy::PolicyNamespace& ns,
   std::string error;
   bool changed = false;
   const policy::Schema* schema = GetPolicySchema();
-  if (schema->Normalize(policy_dict.get(), strategy, &path, &error, &changed)) {
+  if (schema->Normalize(policy_dict, strategy, &path, &error, &changed)) {
     if (changed) {
       LOG(WARNING) << "Unknown (unrecognized or unsupported) policy: " << path
                    << ": " << error;
     }
-    UpdatePolicies(policy_dict.get());
+    return true;
   } else {
     LOG(ERROR) << "Invalid policy contents: " << path << ": " << error;
+    return false;
+  }
+}
+
+namespace {
+void CopyDictionaryValue(const base::DictionaryValue& from,
+                         base::DictionaryValue& to,
+                         std::string key) {
+  const base::Value* value;
+  if (from.Get(key, &value)) {
+    to.Set(key, value->DeepCopy());
+  }
+}
+}  // namespace
+
+scoped_ptr<base::DictionaryValue>
+PolicyWatcher::StoreNewAndReturnChangedPolicies(
+    scoped_ptr<base::DictionaryValue> new_policies) {
+  // Find the changed policies.
+  scoped_ptr<base::DictionaryValue> changed_policies(
+      new base::DictionaryValue());
+  base::DictionaryValue::Iterator iter(*new_policies);
+  while (!iter.IsAtEnd()) {
+    base::Value* old_policy;
+    if (!(old_policies_->Get(iter.key(), &old_policy) &&
+          old_policy->Equals(&iter.value()))) {
+      changed_policies->Set(iter.key(), iter.value().DeepCopy());
+    }
+    iter.Advance();
+  }
+
+  // If one of ThirdPartyAuthConfig policies changed, we need to include all.
+  if (changed_policies->HasKey(key::kRemoteAccessHostTokenUrl) ||
+      changed_policies->HasKey(key::kRemoteAccessHostTokenValidationUrl) ||
+      changed_policies->HasKey(
+          key::kRemoteAccessHostTokenValidationCertificateIssuer)) {
+    CopyDictionaryValue(*new_policies, *changed_policies,
+                        key::kRemoteAccessHostTokenUrl);
+    CopyDictionaryValue(*new_policies, *changed_policies,
+                        key::kRemoteAccessHostTokenValidationUrl);
+    CopyDictionaryValue(*new_policies, *changed_policies,
+                        key::kRemoteAccessHostTokenValidationCertificateIssuer);
+  }
+
+  // Save the new policies.
+  old_policies_.swap(new_policies);
+
+  return changed_policies.Pass();
+}
+
+void PolicyWatcher::OnPolicyUpdated(const policy::PolicyNamespace& ns,
+                                    const policy::PolicyMap& previous,
+                                    const policy::PolicyMap& current) {
+  scoped_ptr<base::DictionaryValue> new_policies =
+      CopyChromotingPoliciesIntoDictionary(current);
+
+  // Check for mistyped values and get rid of unknown policies.
+  if (!NormalizePolicies(new_policies.get())) {
     SignalPolicyError();
+    return;
+  }
+
+  // Use default values for any missing policies.
+  scoped_ptr<base::DictionaryValue> filled_policies =
+      CopyValuesAndAddDefaults(*new_policies, *default_values_);
+
+  // Limit reporting to only the policies that were changed.
+  scoped_ptr<base::DictionaryValue> changed_policies =
+      StoreNewAndReturnChangedPolicies(filled_policies.Pass());
+  if (changed_policies->empty()) {
+    return;
   }
+
+  // Verify that we are calling the callback with valid policies.
+  if (!VerifyWellformedness(*changed_policies)) {
+    SignalPolicyError();
+    return;
+  }
+
+  // Notify our client of the changed policies.
+  policy_updated_callback_.Run(changed_policies.Pass());
 }
 
 void PolicyWatcher::OnPolicyServiceInitialized(policy::PolicyDomain domain) {

remoting/host/policy_watcher.h

@@ -80,13 +80,20 @@ class PolicyWatcher : public policy::PolicyService::Observer,
  private:
   friend class PolicyWatcherTest;
 
-  // Takes the policy dictionary from the OS specific store and extracts the
-  // relevant policies.
-  void UpdatePolicies(const base::DictionaryValue* new_policy);
-
   // Gets Chromoting schema stored inside |owned_schema_registry_|.
   const policy::Schema* GetPolicySchema() const;
 
+  // Simplifying wrapper around Schema::Normalize.
+  // - Returns false if |dict| is invalid (i.e. contains mistyped policy
+  // values).
+  // - Returns true if |dict| was valid or got normalized.
+  bool NormalizePolicies(base::DictionaryValue* dict);
+
+  // Stores |new_policies| into |old_policies_|.  Returns dictionary with items
+  // from |new_policies| that are different from the old |old_policies_|.
+  scoped_ptr<base::DictionaryValue> StoreNewAndReturnChangedPolicies(
+      scoped_ptr<base::DictionaryValue> new_policies);
+
   // Signals policy error to the registered |PolicyErrorCallback|.
   void SignalPolicyError();
 

remoting/host/policy_watcher_unittest.cc

@@ -17,6 +17,8 @@
 
 namespace remoting {
 
+namespace key = ::policy::key;
+
 MATCHER_P(IsPolicies, dict, "") {
   bool equal = arg->Equals(dict);
   if (!equal) {
@@ -56,6 +58,10 @@ class PolicyWatcherTest : public testing::Test {
   PolicyWatcherTest() : message_loop_(base::MessageLoop::TYPE_IO) {}
 
   void SetUp() override {
+    // We expect no callbacks unless explicitly specified by individual tests.
+    EXPECT_CALL(mock_policy_callback_, OnPolicyUpdatePtr(testing::_)).Times(0);
+    EXPECT_CALL(mock_policy_callback_, OnPolicyError()).Times(0);
+
     message_loop_proxy_ = base::MessageLoopProxy::current();
 
     // Retaining a raw pointer to keep control over policy contents.
@@ -63,96 +69,94 @@ class PolicyWatcherTest : public testing::Test {
     policy_watcher_ =
         PolicyWatcher::CreateFromPolicyLoader(make_scoped_ptr(policy_loader_));
 
-    nat_true_.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal, true);
-    nat_false_.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal,
-                          false);
-    nat_one_.SetInteger(policy::key::kRemoteAccessHostFirewallTraversal, 1);
-    domain_empty_.SetString(policy::key::kRemoteAccessHostDomain,
-                            std::string());
-    domain_full_.SetString(policy::key::kRemoteAccessHostDomain, kHostDomain);
+    nat_true_.SetBoolean(key::kRemoteAccessHostFirewallTraversal, true);
+    nat_false_.SetBoolean(key::kRemoteAccessHostFirewallTraversal, false);
+    nat_one_.SetInteger(key::kRemoteAccessHostFirewallTraversal, 1);
+    nat_one_domain_full_.SetInteger(key::kRemoteAccessHostFirewallTraversal, 1);
+    nat_one_domain_full_.SetString(key::kRemoteAccessHostDomain, kHostDomain);
+    domain_empty_.SetString(key::kRemoteAccessHostDomain, std::string());
+    domain_full_.SetString(key::kRemoteAccessHostDomain, kHostDomain);
     SetDefaults(nat_true_others_default_);
-    nat_true_others_default_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, true);
+    nat_true_others_default_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                        true);
     SetDefaults(nat_false_others_default_);
     nat_false_others_default_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, false);
+        key::kRemoteAccessHostFirewallTraversal, false);
     SetDefaults(domain_empty_others_default_);
-    domain_empty_others_default_.SetString(policy::key::kRemoteAccessHostDomain,
+    domain_empty_others_default_.SetString(key::kRemoteAccessHostDomain,
                                            std::string());
     SetDefaults(domain_full_others_default_);
-    domain_full_others_default_.SetString(policy::key::kRemoteAccessHostDomain,
+    domain_full_others_default_.SetString(key::kRemoteAccessHostDomain,
                                           kHostDomain);
-    nat_true_domain_empty_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, true);
-    nat_true_domain_empty_.SetString(policy::key::kRemoteAccessHostDomain,
+    nat_true_domain_empty_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                      true);
+    nat_true_domain_empty_.SetString(key::kRemoteAccessHostDomain,
                                      std::string());
-    nat_true_domain_full_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, true);
-    nat_true_domain_full_.SetString(policy::key::kRemoteAccessHostDomain,
-                                    kHostDomain);
-    nat_false_domain_empty_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, false);
-    nat_false_domain_empty_.SetString(policy::key::kRemoteAccessHostDomain,
+    nat_true_domain_full_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                     true);
+    nat_true_domain_full_.SetString(key::kRemoteAccessHostDomain, kHostDomain);
+    nat_false_domain_empty_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                       false);
+    nat_false_domain_empty_.SetString(key::kRemoteAccessHostDomain,
                                       std::string());
-    nat_false_domain_full_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, false);
-    nat_false_domain_full_.SetString(policy::key::kRemoteAccessHostDomain,
-                                     kHostDomain);
+    nat_false_domain_full_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                      false);
+    nat_false_domain_full_.SetString(key::kRemoteAccessHostDomain, kHostDomain);
     SetDefaults(nat_true_domain_empty_others_default_);
     nat_true_domain_empty_others_default_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, true);
+        key::kRemoteAccessHostFirewallTraversal, true);
     nat_true_domain_empty_others_default_.SetString(
-        policy::key::kRemoteAccessHostDomain, std::string());
+        key::kRemoteAccessHostDomain, std::string());
     unknown_policies_.SetString("UnknownPolicyOne", std::string());
     unknown_policies_.SetString("UnknownPolicyTwo", std::string());
     unknown_policies_.SetBoolean("RemoteAccessHostUnknownPolicyThree", true);
 
     const char kOverrideNatTraversalToFalse[] =
         "{ \"RemoteAccessHostFirewallTraversal\": false }";
-    nat_true_and_overridden_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, true);
+    nat_true_and_overridden_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                        true);
     nat_true_and_overridden_.SetString(
-        policy::key::kRemoteAccessHostDebugOverridePolicies,
+        key::kRemoteAccessHostDebugOverridePolicies,
         kOverrideNatTraversalToFalse);
-    pairing_true_.SetBoolean(policy::key::kRemoteAccessHostAllowClientPairing,
-                             true);
-    pairing_false_.SetBoolean(policy::key::kRemoteAccessHostAllowClientPairing,
+    pairing_true_.SetBoolean(key::kRemoteAccessHostAllowClientPairing, true);
+    pairing_false_.SetBoolean(key::kRemoteAccessHostAllowClientPairing, false);
+    gnubby_auth_true_.SetBoolean(key::kRemoteAccessHostAllowGnubbyAuth, true);
+    gnubby_auth_false_.SetBoolean(key::kRemoteAccessHostAllowGnubbyAuth, false);
+    relay_true_.SetBoolean(key::kRemoteAccessHostAllowRelayedConnection, true);
+    relay_false_.SetBoolean(key::kRemoteAccessHostAllowRelayedConnection,
                             false);
-    gnubby_auth_true_.SetBoolean(policy::key::kRemoteAccessHostAllowGnubbyAuth,
-                                 true);
-    gnubby_auth_false_.SetBoolean(policy::key::kRemoteAccessHostAllowGnubbyAuth,
-                                  false);
-    relay_true_.SetBoolean(policy::key::kRemoteAccessHostAllowRelayedConnection,
-                           true);
-    relay_false_.SetBoolean(
-        policy::key::kRemoteAccessHostAllowRelayedConnection, false);
-    port_range_full_.SetString(policy::key::kRemoteAccessHostUdpPortRange,
-                               kPortRange);
-    port_range_empty_.SetString(policy::key::kRemoteAccessHostUdpPortRange,
+    port_range_full_.SetString(key::kRemoteAccessHostUdpPortRange, kPortRange);
+    port_range_empty_.SetString(key::kRemoteAccessHostUdpPortRange,
                                 std::string());
-    curtain_true_.SetBoolean(policy::key::kRemoteAccessHostRequireCurtain,
-                             true);
-    curtain_false_.SetBoolean(policy::key::kRemoteAccessHostRequireCurtain,
-                              false);
-    username_true_.SetBoolean(policy::key::kRemoteAccessHostMatchUsername,
-                              true);
-    username_false_.SetBoolean(policy::key::kRemoteAccessHostMatchUsername,
-                               false);
-    talk_gadget_blah_.SetString(policy::key::kRemoteAccessHostTalkGadgetPrefix,
-                                "blah");
-    token_url_https_.SetString(policy::key::kRemoteAccessHostTalkGadgetPrefix,
-                               "https://example.com");
-    token_validation_url_https_.SetString(
-        policy::key::kRemoteAccessHostTalkGadgetPrefix, "https://example.com");
-    token_certificate_blah_.SetString(
-        policy::key::kRemoteAccessHostTokenValidationCertificateIssuer, "blah");
+    port_range_malformed_.SetString(key::kRemoteAccessHostUdpPortRange,
+                                    "malformed");
+    port_range_malformed_domain_full_.MergeDictionary(&port_range_malformed_);
+    port_range_malformed_domain_full_.SetString(key::kRemoteAccessHostDomain,
+                                                kHostDomain);
+
+    curtain_true_.SetBoolean(key::kRemoteAccessHostRequireCurtain, true);
+    curtain_false_.SetBoolean(key::kRemoteAccessHostRequireCurtain, false);
+    username_true_.SetBoolean(key::kRemoteAccessHostMatchUsername, true);
+    username_false_.SetBoolean(key::kRemoteAccessHostMatchUsername, false);
+    talk_gadget_blah_.SetString(key::kRemoteAccessHostTalkGadgetPrefix, "blah");
+    third_party_auth_partial_.SetString(key::kRemoteAccessHostTokenUrl,
+                                        "https://token.com");
+    third_party_auth_partial_.SetString(
+        key::kRemoteAccessHostTokenValidationUrl, "https://validation.com");
+    third_party_auth_full_.MergeDictionary(&third_party_auth_partial_);
+    third_party_auth_full_.SetString(
+        key::kRemoteAccessHostTokenValidationCertificateIssuer,
+        "certificate subject");
+    third_party_auth_cert_empty_.MergeDictionary(&third_party_auth_partial_);
+    third_party_auth_cert_empty_.SetString(
+        key::kRemoteAccessHostTokenValidationCertificateIssuer, "");
 
 #if !defined(NDEBUG)
     SetDefaults(nat_false_overridden_others_default_);
     nat_false_overridden_others_default_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, false);
+        key::kRemoteAccessHostFirewallTraversal, false);
     nat_false_overridden_others_default_.SetString(
-        policy::key::kRemoteAccessHostDebugOverridePolicies,
+        key::kRemoteAccessHostDebugOverridePolicies,
         kOverrideNatTraversalToFalse);
 #endif
   }
@@ -214,6 +218,7 @@ class PolicyWatcherTest : public testing::Test {
   base::DictionaryValue nat_true_;
   base::DictionaryValue nat_false_;
   base::DictionaryValue nat_one_;
+  base::DictionaryValue nat_one_domain_full_;
   base::DictionaryValue domain_empty_;
   base::DictionaryValue domain_full_;
   base::DictionaryValue nat_true_others_default_;
@@ -236,35 +241,34 @@ class PolicyWatcherTest : public testing::Test {
   base::DictionaryValue relay_false_;
   base::DictionaryValue port_range_full_;
   base::DictionaryValue port_range_empty_;
+  base::DictionaryValue port_range_malformed_;
+  base::DictionaryValue port_range_malformed_domain_full_;
   base::DictionaryValue curtain_true_;
   base::DictionaryValue curtain_false_;
   base::DictionaryValue username_true_;
   base::DictionaryValue username_false_;
   base::DictionaryValue talk_gadget_blah_;
-  base::DictionaryValue token_url_https_;
-  base::DictionaryValue token_validation_url_https_;
-  base::DictionaryValue token_certificate_blah_;
+  base::DictionaryValue third_party_auth_full_;
+  base::DictionaryValue third_party_auth_partial_;
+  base::DictionaryValue third_party_auth_cert_empty_;
 
  private:
   void SetDefaults(base::DictionaryValue& dict) {
-    dict.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal, true);
-    dict.SetBoolean(policy::key::kRemoteAccessHostAllowRelayedConnection, true);
-    dict.SetString(policy::key::kRemoteAccessHostUdpPortRange, "");
-    dict.SetString(policy::key::kRemoteAccessHostDomain, std::string());
-    dict.SetBoolean(policy::key::kRemoteAccessHostMatchUsername, false);
-    dict.SetString(policy::key::kRemoteAccessHostTalkGadgetPrefix,
+    dict.SetBoolean(key::kRemoteAccessHostFirewallTraversal, true);
+    dict.SetBoolean(key::kRemoteAccessHostAllowRelayedConnection, true);
+    dict.SetString(key::kRemoteAccessHostUdpPortRange, "");
+    dict.SetString(key::kRemoteAccessHostDomain, std::string());
+    dict.SetBoolean(key::kRemoteAccessHostMatchUsername, false);
+    dict.SetString(key::kRemoteAccessHostTalkGadgetPrefix,
                    kDefaultHostTalkGadgetPrefix);
-    dict.SetBoolean(policy::key::kRemoteAccessHostRequireCurtain, false);
-    dict.SetString(policy::key::kRemoteAccessHostTokenUrl, std::string());
-    dict.SetString(policy::key::kRemoteAccessHostTokenValidationUrl,
-                   std::string());
-    dict.SetString(
-        policy::key::kRemoteAccessHostTokenValidationCertificateIssuer,
-        std::string());
-    dict.SetBoolean(policy::key::kRemoteAccessHostAllowClientPairing, true);
-    dict.SetBoolean(policy::key::kRemoteAccessHostAllowGnubbyAuth, true);
+    dict.SetBoolean(key::kRemoteAccessHostRequireCurtain, false);
+    dict.SetString(key::kRemoteAccessHostTokenUrl, "");
+    dict.SetString(key::kRemoteAccessHostTokenValidationUrl, "");
+    dict.SetString(key::kRemoteAccessHostTokenValidationCertificateIssuer, "");
+    dict.SetBoolean(key::kRemoteAccessHostAllowClientPairing, true);
+    dict.SetBoolean(key::kRemoteAccessHostAllowGnubbyAuth, true);
 #if !defined(NDEBUG)
-    dict.SetString(policy::key::kRemoteAccessHostDebugOverridePolicies, "");
+    dict.SetString(key::kRemoteAccessHostDebugOverridePolicies, "");
 #endif
 
     ASSERT_THAT(&dict, IsPolicies(&GetDefaultValues()))
@@ -307,6 +311,26 @@ TEST_F(PolicyWatcherTest, NatWrongType) {
   StartWatching();
 }
 
+// This test verifies that a mistyped policy value is still detected
+// even though it doesn't change during the second SetPolicies call.
+TEST_F(PolicyWatcherTest, NatWrongTypeThenIrrelevantChange) {
+  EXPECT_CALL(mock_policy_callback_, OnPolicyError()).Times(2);
+
+  SetPolicies(nat_one_);
+  StartWatching();
+  SetPolicies(nat_one_domain_full_);
+}
+
+// This test verifies that a malformed policy value is still detected
+// even though it doesn't change during the second SetPolicies call.
+TEST_F(PolicyWatcherTest, PortRangeMalformedThenIrrelevantChange) {
+  EXPECT_CALL(mock_policy_callback_, OnPolicyError()).Times(2);
+
+  SetPolicies(port_range_malformed_);
+  StartWatching();
+  SetPolicies(port_range_malformed_domain_full_);
+}
+
 TEST_F(PolicyWatcherTest, DomainEmpty) {
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&domain_empty_others_default_)));
@@ -521,40 +545,36 @@ TEST_F(PolicyWatcherTest, TalkGadgetPrefix) {
   SetPolicies(talk_gadget_blah_);
 }
 
-TEST_F(PolicyWatcherTest, TokenUrl) {
+TEST_F(PolicyWatcherTest, ThirdPartyAuthFull) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&token_url_https_)));
+              OnPolicyUpdatePtr(IsPolicies(&third_party_auth_full_)));
 
   SetPolicies(empty_);
   StartWatching();
-  SetPolicies(token_url_https_);
+  SetPolicies(third_party_auth_full_);
 }
 
-TEST_F(PolicyWatcherTest, TokenValidationUrl) {
+// This test verifies what happens when only 1 out of 3 third-party auth
+// policies changes.  Without the other 2 policy values such policy values
+// combination is invalid (i.e. cannot have TokenUrl without
+// TokenValidationUrl) and can trigger OnPolicyError unless PolicyWatcher
+// implementation is careful around this scenario.
+TEST_F(PolicyWatcherTest, ThirdPartyAuthPartialToFull) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&token_validation_url_https_)));
-
-  SetPolicies(empty_);
-  StartWatching();
-  SetPolicies(token_validation_url_https_);
-}
-
-TEST_F(PolicyWatcherTest, TokenValidationCertificateIssuer) {
-  testing::InSequence sequence;
-  EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
+              OnPolicyUpdatePtr(IsPolicies(&third_party_auth_cert_empty_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&token_certificate_blah_)));
+              OnPolicyUpdatePtr(IsPolicies(&third_party_auth_full_)));
 
   SetPolicies(empty_);
   StartWatching();
-  SetPolicies(token_certificate_blah_);
+  SetPolicies(third_party_auth_partial_);
+  SetPolicies(third_party_auth_full_);
 }
 
 TEST_F(PolicyWatcherTest, UdpPortRange) {
@@ -587,13 +607,13 @@ TEST_F(PolicyWatcherTest, PolicySchemaAndPolicyWatcherShouldBeInSync) {
 #if defined(OS_WIN)
   // RemoteAccessHostMatchUsername is marked in policy_templates.json as not
   // supported on Windows and therefore is (by design) excluded from the schema.
-  expected_schema.erase(policy::key::kRemoteAccessHostMatchUsername);
+  expected_schema.erase(key::kRemoteAccessHostMatchUsername);
 #endif
 #if defined(NDEBUG)
   // Policy schema / policy_templates.json cannot differ between debug and
   // release builds so we compensate below to account for the fact that
   // PolicyWatcher::default_values_ does differ between debug and release.
-  expected_schema[policy::key::kRemoteAccessHostDebugOverridePolicies] =
+  expected_schema[key::kRemoteAccessHostDebugOverridePolicies] =
       base::Value::TYPE_STRING;
 #endif
 

remoting/host/remoting_me2me_host.cc

@@ -64,6 +64,7 @@
 #include "remoting/host/shutdown_watchdog.h"
 #include "remoting/host/signaling_connector.h"
 #include "remoting/host/single_window_desktop_environment.h"
+#include "remoting/host/third_party_auth_config.h"
 #include "remoting/host/token_validator_factory_impl.h"
 #include "remoting/host/usage_stats_consent.h"
 #include "remoting/host/username.h"
@@ -71,6 +72,7 @@
 #include "remoting/protocol/me2me_host_authenticator_factory.h"
 #include "remoting/protocol/network_settings.h"
 #include "remoting/protocol/pairing_registry.h"
+#include "remoting/protocol/port_range.h"
 #include "remoting/protocol/token_validator.h"
 #include "remoting/signaling/xmpp_signal_strategy.h"
 
@@ -339,8 +341,7 @@ class HostProcess : public ConfigWatcher::Delegate,
   bool host_username_match_required_;
   bool allow_nat_traversal_;
   bool allow_relay_;
-  uint16 min_udp_port_;
-  uint16 max_udp_port_;
+  PortRange udp_port_range_;
   std::string talkgadget_prefix_;
   bool allow_pairing_;
 
@@ -394,8 +395,6 @@ HostProcess::HostProcess(scoped_ptr<ChromotingHostContext> context,
       host_username_match_required_(false),
       allow_nat_traversal_(true),
       allow_relay_(true),
-      min_udp_port_(0),
-      max_udp_port_(0),
       allow_pairing_(true),
       curtain_required_(false),
       enable_gnubby_auth_(false),
@@ -672,7 +671,7 @@ void HostProcess::CreateAuthenticatorFactory() {
 
   scoped_ptr<protocol::AuthenticatorFactory> factory;
 
-  if (third_party_auth_config_.is_empty()) {
+  if (third_party_auth_config_.is_null()) {
     scoped_refptr<PairingRegistry> pairing_registry;
     if (allow_pairing_) {
       // On Windows |pairing_registry_| is initialized in
@@ -696,7 +695,10 @@ void HostProcess::CreateAuthenticatorFactory() {
         host_secret_hash_, pairing_registry);
 
     host_->set_pairing_registry(pairing_registry);
-  } else if (third_party_auth_config_.is_valid()) {
+  } else {
+    DCHECK(third_party_auth_config_.token_url.is_valid());
+    DCHECK(third_party_auth_config_.token_validation_url.is_valid());
+
     scoped_ptr<protocol::TokenValidatorFactory> token_validator_factory(
         new TokenValidatorFactoryImpl(
             third_party_auth_config_,
@@ -704,17 +706,6 @@ void HostProcess::CreateAuthenticatorFactory() {
     factory = protocol::Me2MeHostAuthenticatorFactory::CreateWithThirdPartyAuth(
         use_service_account_, host_owner_, local_certificate, key_pair_,
         token_validator_factory.Pass());
-
-  } else {
-    // TODO(rmsousa): If the policy is bad the host should not go online. It
-    // should keep running, but not connected, until the policies are fixed.
-    // Having it show up as online and then reject all clients is misleading.
-    LOG(ERROR) << "One of the third-party token URLs is empty or invalid. "
-               << "Host will reject all clients until policies are corrected. "
-               << "TokenUrl: " << third_party_auth_config_.token_url << ", "
-               << "TokenValidationUrl: "
-               << third_party_auth_config_.token_validation_url;
-    factory = protocol::Me2MeHostAuthenticatorFactory::CreateRejecting();
   }
 
 #if defined(OS_POSIX)
@@ -1189,34 +1180,15 @@ bool HostProcess::OnUdpPortPolicyUpdate(base::DictionaryValue* policies) {
   // Returns true if the host has to be restarted after this policy update.
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
 
-  std::string udp_port_range;
+  std::string string_value;
   if (!policies->GetString(policy::key::kRemoteAccessHostUdpPortRange,
-                           &udp_port_range)) {
+                           &string_value)) {
     return false;
   }
 
-  // Use default values if policy setting is empty or invalid.
-  uint16 min_udp_port = 0;
-  uint16 max_udp_port = 0;
-  if (!udp_port_range.empty() &&
-      !NetworkSettings::ParsePortRange(udp_port_range, &min_udp_port,
-                                       &max_udp_port)) {
-    LOG(WARNING) << "Invalid port range policy: \"" << udp_port_range
-                 << "\". Using default values.";
-  }
-
-  if (min_udp_port_ != min_udp_port || max_udp_port_ != max_udp_port) {
-    if (min_udp_port != 0 && max_udp_port != 0) {
-      HOST_LOG << "Policy restricts UDP port range to [" << min_udp_port
-               << ", " << max_udp_port << "]";
-    } else {
-      HOST_LOG << "Policy does not restrict UDP port range.";
-    }
-    min_udp_port_ = min_udp_port;
-    max_udp_port_ = max_udp_port;
+  DCHECK(PortRange::Parse(string_value, &udp_port_range_));
+  HOST_LOG << "Policy restricts UDP port range to: " << udp_port_range_;
   return true;
-  }
-  return false;
 }
 
 bool HostProcess::OnCurtainPolicyUpdate(base::DictionaryValue* policies) {
@@ -1274,39 +1246,18 @@ bool HostProcess::OnHostTalkGadgetPrefixPolicyUpdate(
 }
 
 bool HostProcess::OnHostTokenUrlPolicyUpdate(base::DictionaryValue* policies) {
-  // Returns true if the host has to be restarted after this policy update.
-  DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
-
-  bool token_policy_changed = false;
-  std::string token_url_string;
-  if (policies->GetString(policy::key::kRemoteAccessHostTokenUrl,
-                          &token_url_string)) {
-    token_policy_changed = true;
-    third_party_auth_config_.token_url = GURL(token_url_string);
-  }
-  std::string token_validation_url_string;
-  if (policies->GetString(policy::key::kRemoteAccessHostTokenValidationUrl,
-                          &token_validation_url_string)) {
-    token_policy_changed = true;
-    third_party_auth_config_.token_validation_url =
-        GURL(token_validation_url_string);
-  }
-  if (policies->GetString(
-          policy::key::kRemoteAccessHostTokenValidationCertificateIssuer,
-          &third_party_auth_config_.token_validation_cert_issuer)) {
-    token_policy_changed = true;
-  }
-
-  if (token_policy_changed) {
+  switch (ThirdPartyAuthConfig::Parse(*policies, &third_party_auth_config_)) {
+    case ThirdPartyAuthConfig::NoPolicy:
+      return false;
+    case ThirdPartyAuthConfig::ParsingSuccess:
       HOST_LOG << "Policy sets third-party token URLs: "
-             << "TokenUrl: "
-             << third_party_auth_config_.token_url << ", "
-             << "TokenValidationUrl: "
-             << third_party_auth_config_.token_validation_url << ", "
-             << "TokenValidationCertificateIssuer: "
-             << third_party_auth_config_.token_validation_cert_issuer;
-  }
-  return token_policy_changed;
+               << third_party_auth_config_;
+      return true;
+    case ThirdPartyAuthConfig::InvalidPolicy:
+    default:
+      NOTREACHED();
+      return false;
+  }
 }
 
 bool HostProcess::OnPairingPolicyUpdate(base::DictionaryValue* policies) {
@@ -1392,15 +1343,14 @@ void HostProcess::StartHost() {
 
   NetworkSettings network_settings(network_flags);
 
-  if (min_udp_port_ && max_udp_port_) {
-    network_settings.min_port = min_udp_port_;
-    network_settings.max_port = max_udp_port_;
+  if (!udp_port_range_.is_null()) {
+    network_settings.port_range = udp_port_range_;
   } else if (!allow_nat_traversal_) {
     // For legacy reasons we have to restrict the port range to a set of default
     // values when nat traversal is disabled, even if the port range was not
     // set in policy.
-    network_settings.min_port = NetworkSettings::kDefaultMinPort;
-    network_settings.max_port = NetworkSettings::kDefaultMaxPort;
+    network_settings.port_range.min_port = NetworkSettings::kDefaultMinPort;
+    network_settings.port_range.max_port = NetworkSettings::kDefaultMaxPort;
   }
 
   host_.reset(new ChromotingHost(

remoting/host/third_party_auth_config.cc

+// Copyright 2015 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/third_party_auth_config.h"
+
+#include "base/logging.h"
+#include "base/values.h"
+#include "policy/policy_constants.h"
+
+namespace remoting {
+
+namespace {
+
+bool ParseUrlPolicy(const std::string& str, GURL* out) {
+  if (str.empty()) {
+    *out = GURL();
+    return true;
+  }
+
+  GURL gurl(str);
+  if (!gurl.is_valid()) {
+    LOG(ERROR) << "Not a valid URL: " << str;
+    return false;
+  }
+// We validate https-vs-http only on Release builds to help with manual testing.
+#if defined(NDEBUG)
+  if (!gurl.SchemeIsSecure()) {
+    LOG(ERROR) << "Not a secure URL: " << str;
+    return false;
+  }
+#endif
+
+  *out = gurl;
+  return true;
+}
+
+}  // namespace
+
+bool ThirdPartyAuthConfig::ParseStrings(
+    const std::string& token_url,
+    const std::string& token_validation_url,
+    const std::string& token_validation_cert_issuer,
+    ThirdPartyAuthConfig* result) {
+  ThirdPartyAuthConfig tmp;
+
+  // Extract raw values for the 3 individual fields.
+  bool urls_valid = true;
+  urls_valid &= ParseUrlPolicy(token_url, &tmp.token_url);
+  urls_valid &= ParseUrlPolicy(token_validation_url, &tmp.token_validation_url);
+  if (!urls_valid) {
+    return false;
+  }
+  tmp.token_validation_cert_issuer = token_validation_cert_issuer;
+
+  // Validate inter-dependencies between the 3 fields.
+  if (tmp.token_url.is_empty() ^ tmp.token_validation_url.is_empty()) {
+    LOG(ERROR) << "TokenUrl and TokenValidationUrl "
+               << "have to be specified together.";
+    return false;
+  }
+  if (!tmp.token_validation_cert_issuer.empty() && tmp.token_url.is_empty()) {
+    LOG(ERROR) << "TokenValidationCertificateIssuer cannot be used "
+               << "without TokenUrl and TokenValidationUrl.";
+    return false;
+  }
+
+  *result = tmp;
+  return true;
+}
+
+namespace {
+
+void ExtractHelper(const base::DictionaryValue& policy_dict,
+                   const std::string& policy_name,
+                   bool* policy_present,
+                   std::string* policy_value) {
+  if (policy_dict.GetString(policy_name, policy_value)) {
+    *policy_present = true;
+  } else {
+    policy_value->clear();
+  }
+}
+
+}  // namespace
+
+bool ThirdPartyAuthConfig::ExtractStrings(
+    const base::DictionaryValue& policy_dict,
+    std::string* token_url,
+    std::string* token_validation_url,
+    std::string* token_validation_cert_issuer) {
+  bool policies_present = false;
+  ExtractHelper(policy_dict, policy::key::kRemoteAccessHostTokenUrl,
+                &policies_present, token_url);
+  ExtractHelper(policy_dict, policy::key::kRemoteAccessHostTokenValidationUrl,
+                &policies_present, token_validation_url);
+  ExtractHelper(policy_dict,
+                policy::key::kRemoteAccessHostTokenValidationCertificateIssuer,
+                &policies_present, token_validation_cert_issuer);
+  return policies_present;
+}
+
+ThirdPartyAuthConfig::ParseStatus ThirdPartyAuthConfig::Parse(
+    const base::DictionaryValue& policy_dict,
+    ThirdPartyAuthConfig* result) {
+  // Extract 3 individial policy values.
+  std::string token_url;
+  std::string token_validation_url;
+  std::string token_validation_cert_issuer;
+  if (!ThirdPartyAuthConfig::ExtractStrings(policy_dict, &token_url,
+                                            &token_validation_url,
+                                            &token_validation_cert_issuer)) {
+    return NoPolicy;
+  }
+
+  // Parse the policy value.
+  if (!ThirdPartyAuthConfig::ParseStrings(token_url, token_validation_url,
+                                          token_validation_cert_issuer,
+                                          result)) {
+    return InvalidPolicy;
+  }
+
+  return ParsingSuccess;
+}
+
+std::ostream& operator<<(std::ostream& os, const ThirdPartyAuthConfig& cfg) {
+  if (cfg.is_null()) {
+    os << "<no 3rd party auth config specified>";
+  } else {
+    os << "TokenUrl = <" << cfg.token_url << ">, ";
+    os << "TokenValidationUrl = <" << cfg.token_validation_url << ">, ";
+    os << "TokenValidationCertificateIssuer = <"
+       << cfg.token_validation_cert_issuer << ">";
+  }
+  return os;
+}
+
+}  // namespace remoting

remoting/host/third_party_auth_config.h

+// Copyright 2015 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.
+
+#ifndef REMOTING_HOST_THIRD_PARTY_AUTH_CONFIG_H_
+#define REMOTING_HOST_THIRD_PARTY_AUTH_CONFIG_H_
+
+#include <ostream>
+#include <string>
+
+#include "base/gtest_prod_util.h"
+#include "url/gurl.h"
+
+namespace base {
+class DictionaryValue;
+}  // namespace base
+
+namespace remoting {
+
+struct ThirdPartyAuthConfig {
+  GURL token_url;
+  GURL token_validation_url;
+  std::string token_validation_cert_issuer;
+
+  inline bool is_null() const {
+    return token_url.is_empty() && token_validation_url.is_empty();
+  }
+
+  // Status of Parse method call.
+  enum ParseStatus {
+    // |policy_dict| contains invalid entries (i.e. malformed urls).
+    // |result| has not been modified.
+    InvalidPolicy,
+
+    // |policy_dict| doesn't contain any ThirdPartyAuthConfig-related entries.
+    // |result| has not been modified.
+    NoPolicy,
+
+    // |policy_dict| contains valid entries that have been stored into |result|.
+    ParsingSuccess,
+  };
+  static ParseStatus Parse(const base::DictionaryValue& policy_dict,
+                           ThirdPartyAuthConfig* result);
+
+ private:
+  // Returns false and doesn't modify |result| if parsing fails (i.e. some input
+  // values are invalid).
+  static bool ParseStrings(const std::string& token_url,
+                           const std::string& token_validation_url,
+                           const std::string& token_validation_cert_issuer,
+                           ThirdPartyAuthConfig* result);
+  FRIEND_TEST_ALL_PREFIXES(InvalidUrlTest, ParseInvalidUrl);
+  FRIEND_TEST_ALL_PREFIXES(ThirdPartyAuthConfig, ParseEmpty);
+  FRIEND_TEST_ALL_PREFIXES(ThirdPartyAuthConfig, ParseValidAll);
+  FRIEND_TEST_ALL_PREFIXES(ThirdPartyAuthConfig, ParseValidNoCert);
+  FRIEND_TEST_ALL_PREFIXES(ThirdPartyAuthConfig, ParseInvalidCombination);
+  FRIEND_TEST_ALL_PREFIXES(ThirdPartyAuthConfig, ParseHttp);
+
+  // Extracts raw (raw = as strings) policy values from |policy_dict|.
+  // Missing policy values are set to an empty string.
+  // Returns false if no ThirdPartyAuthConfig-related policies were present.
+  static bool ExtractStrings(const base::DictionaryValue& policy_dict,
+                             std::string* token_url,
+                             std::string* token_validation_url,
+                             std::string* token_validation_cert_issuer);
+  FRIEND_TEST_ALL_PREFIXES(ThirdPartyAuthConfig, ExtractEmpty);
+  FRIEND_TEST_ALL_PREFIXES(ThirdPartyAuthConfig, ExtractUnknown);
+  FRIEND_TEST_ALL_PREFIXES(ThirdPartyAuthConfig, ExtractAll);
+  FRIEND_TEST_ALL_PREFIXES(ThirdPartyAuthConfig, ExtractPartial);
+};
+
+std::ostream& operator<<(std::ostream& os, const ThirdPartyAuthConfig& cfg);
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_THIRD_PARTY_AUTH_CONFIG_H_

remoting/host/third_party_auth_config_unittest.cc

+// Copyright 2015 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/third_party_auth_config.h"
+
+#include <sstream>
+
+#include "base/values.h"
+#include "policy/policy_constants.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace remoting {
+
+namespace key = ::policy::key;
+
+TEST(ThirdPartyAuthConfig, ParseEmpty) {
+  ThirdPartyAuthConfig third_party_auth_config;
+
+  EXPECT_TRUE(
+      ThirdPartyAuthConfig::ParseStrings("", "", "", &third_party_auth_config));
+  EXPECT_TRUE(third_party_auth_config.is_null());
+}
+
+TEST(ThirdPartyAuthConfig, ParseValidAll) {
+  ThirdPartyAuthConfig third_party_auth_config;
+
+  EXPECT_TRUE(ThirdPartyAuthConfig::ParseStrings(
+      "https://token.com/", "https://validation.com/", "certificate subject",
+      &third_party_auth_config));
+  EXPECT_FALSE(third_party_auth_config.is_null());
+  EXPECT_EQ("https://token.com/", third_party_auth_config.token_url.spec());
+  EXPECT_EQ("https://validation.com/",
+            third_party_auth_config.token_validation_url.spec());
+  EXPECT_EQ("certificate subject",
+            third_party_auth_config.token_validation_cert_issuer);
+}
+
+TEST(ThirdPartyAuthConfig, ParseValidNoCert) {
+  ThirdPartyAuthConfig third_party_auth_config;
+
+  EXPECT_TRUE(ThirdPartyAuthConfig::ParseStrings("https://token.com/",
+                                                 "https://validation.com/", "",
+                                                 &third_party_auth_config));
+  EXPECT_FALSE(third_party_auth_config.is_null());
+  EXPECT_EQ("https://token.com/", third_party_auth_config.token_url.spec());
+  EXPECT_EQ("https://validation.com/",
+            third_party_auth_config.token_validation_url.spec());
+  EXPECT_EQ("", third_party_auth_config.token_validation_cert_issuer);
+}
+
+// We validate https-vs-http only on Release builds to help with manual testing.
+#if !defined(NDEBUG)
+TEST(ThirdPartyAuthConfig, ParseHttp) {
+  ThirdPartyAuthConfig third_party_auth_config;
+
+  EXPECT_TRUE(ThirdPartyAuthConfig::ParseStrings("http://token.com/",
+                                                 "http://validation.com/", "",
+                                                 &third_party_auth_config));
+  EXPECT_FALSE(third_party_auth_config.is_null());
+  EXPECT_EQ("http://token.com/", third_party_auth_config.token_url.spec());
+  EXPECT_EQ("http://validation.com/",
+            third_party_auth_config.token_validation_url.spec());
+}
+#endif
+
+namespace {
+
+const std::string valid_url("https://valid.com");
+const std::string valid_cert("certificate subject");
+
+}  // namespace
+
+class InvalidUrlTest : public ::testing::TestWithParam<const char*> {};
+
+TEST_P(InvalidUrlTest, ParseInvalidUrl) {
+  const std::string& invalid_url(GetParam());
+
+  // Populate |config| with some known data - we will use this for validating
+  // that |config| doesn't get modified by ParseStrings during subsequent
+  // parsing
+  // failure tests.
+  ThirdPartyAuthConfig config;
+  EXPECT_TRUE(ThirdPartyAuthConfig::ParseStrings(
+      "https://token.com/", "https://validation.com/", "certificate subject",
+      &config));
+
+  // Test for parsing failure.
+  EXPECT_FALSE(ThirdPartyAuthConfig::ParseStrings(invalid_url, valid_url,
+                                                  valid_cert, &config));
+  EXPECT_FALSE(ThirdPartyAuthConfig::ParseStrings(valid_url, invalid_url,
+                                                  valid_cert, &config));
+  EXPECT_FALSE(
+      ThirdPartyAuthConfig::ParseStrings(invalid_url, "", "", &config));
+  EXPECT_FALSE(
+      ThirdPartyAuthConfig::ParseStrings("", invalid_url, "", &config));
+
+  // Validate that ParseStrings doesn't modify its output upon parsing failure.
+  EXPECT_EQ("https://token.com/", config.token_url.spec());
+  EXPECT_EQ("https://validation.com/", config.token_validation_url.spec());
+  EXPECT_EQ("certificate subject", config.token_validation_cert_issuer);
+}
+
+// We validate https-vs-http only on Release builds to help with manual testing.
+#if defined(NDEBUG)
+INSTANTIATE_TEST_CASE_P(ThirdPartyAuthConfig,
+                        InvalidUrlTest,
+                        ::testing::Values("http://insecure.com",
+                                          "I am not a URL"));
+#else
+INSTANTIATE_TEST_CASE_P(ThirdPartyAuthConfig,
+                        InvalidUrlTest,
+                        ::testing::Values("I am not a URL"));
+#endif
+
+TEST(ThirdPartyAuthConfig, ParseInvalidCombination) {
+  // Populate |config| with some known data - we will use this for validating
+  // that |config| doesn't get modified by ParseStrings during subsequent
+  // parsing
+  // failure tests.
+  ThirdPartyAuthConfig config;
+  EXPECT_TRUE(ThirdPartyAuthConfig::ParseStrings(
+      "https://token.com/", "https://validation.com/", "certificate subject",
+      &config));
+
+  // Only one of TokenUrl and TokenValidationUrl
+  EXPECT_FALSE(
+      ThirdPartyAuthConfig::ParseStrings("", valid_url, valid_cert, &config));
+  EXPECT_FALSE(
+      ThirdPartyAuthConfig::ParseStrings(valid_url, "", valid_cert, &config));
+
+  // CertSubject when no TokenUrl and TokenValidationUrl
+  EXPECT_FALSE(ThirdPartyAuthConfig::ParseStrings("", "", valid_cert, &config));
+
+  // Validate that ParseStrings doesn't modify its output upon parsing failure.
+  EXPECT_EQ("https://token.com/", config.token_url.spec());
+  EXPECT_EQ("https://validation.com/", config.token_validation_url.spec());
+  EXPECT_EQ("certificate subject", config.token_validation_cert_issuer);
+}
+
+TEST(ThirdPartyAuthConfig, ExtractEmpty) {
+  base::DictionaryValue dict;
+  std::string url1, url2, cert;
+  EXPECT_FALSE(ThirdPartyAuthConfig::ExtractStrings(dict, &url1, &url2, &cert));
+}
+
+TEST(ThirdPartyAuthConfig, ExtractUnknown) {
+  base::DictionaryValue dict;
+  dict.SetString("unknownName", "someValue");
+
+  std::string url1, url2, cert;
+  EXPECT_FALSE(ThirdPartyAuthConfig::ExtractStrings(dict, &url1, &url2, &cert));
+}
+
+TEST(ThirdPartyAuthConfig, ExtractAll) {
+  base::DictionaryValue dict;
+  dict.SetString(key::kRemoteAccessHostTokenUrl, "test1");
+  dict.SetString(key::kRemoteAccessHostTokenValidationUrl, "test2");
+  dict.SetString(key::kRemoteAccessHostTokenValidationCertificateIssuer, "t3");
+
+  std::string url1, url2, cert;
+  EXPECT_TRUE(ThirdPartyAuthConfig::ExtractStrings(dict, &url1, &url2, &cert));
+  EXPECT_EQ("test1", url1);
+  EXPECT_EQ("test2", url2);
+  EXPECT_EQ("t3", cert);
+}
+
+TEST(ThirdPartyAuthConfig, ExtractPartial) {
+  base::DictionaryValue dict;
+  dict.SetString(key::kRemoteAccessHostTokenValidationUrl, "test2");
+
+  std::string url1, url2, cert;
+  EXPECT_TRUE(ThirdPartyAuthConfig::ExtractStrings(dict, &url1, &url2, &cert));
+  EXPECT_EQ("", url1);
+  EXPECT_EQ("test2", url2);
+  EXPECT_EQ("", cert);
+}
+
+TEST(ThirdPartyAuthConfig, Output) {
+  ThirdPartyAuthConfig third_party_auth_config;
+  third_party_auth_config.token_url = GURL("https://token.com");
+  third_party_auth_config.token_validation_url = GURL("https://validation.com");
+  third_party_auth_config.token_validation_cert_issuer = "certificate subject";
+
+  std::ostringstream str;
+  str << third_party_auth_config;
+
+  EXPECT_THAT(str.str(), testing::HasSubstr("token"));
+  EXPECT_THAT(str.str(), testing::HasSubstr("validation"));
+  EXPECT_THAT(str.str(), testing::HasSubstr("certificate subject"));
+}
+
+}  // namespace remoting

remoting/host/token_validator_base.h

@@ -9,6 +9,7 @@
 #include "base/memory/weak_ptr.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "remoting/host/third_party_auth_config.h"
 #include "remoting/protocol/token_validator.h"
 #include "url/gurl.h"
 
@@ -19,20 +20,6 @@ typedef std::vector<scoped_refptr<X509Certificate> > CertificateList;
 
 namespace remoting {
 
-struct ThirdPartyAuthConfig {
-  inline bool is_empty() const {
-    return token_url.is_empty() && token_validation_url.is_empty();
-  }
-
-  inline bool is_valid() const {
-    return token_url.is_valid() && token_validation_url.is_valid();
-  }
-
-  GURL token_url;
-  GURL token_validation_url;
-  std::string token_validation_cert_issuer;
-};
-
 class TokenValidatorBase
     : public net::URLRequest::Delegate,
       public protocol::TokenValidator {

remoting/protocol/chromium_port_allocator.cc

@@ -156,8 +156,8 @@ scoped_ptr<ChromiumPortAllocator> ChromiumPortAllocator::Create(
     flags |= cricket::PORTALLOCATOR_DISABLE_RELAY;
 
   result->set_flags(flags);
-  result->SetPortRange(network_settings.min_port,
-                       network_settings.max_port);
+  result->SetPortRange(network_settings.port_range.min_port,
+                       network_settings.port_range.max_port);
 
   return result.Pass();
 }

remoting/protocol/me2me_host_authenticator_factory.cc

@@ -97,12 +97,6 @@ Me2MeHostAuthenticatorFactory::CreateWithThirdPartyAuth(
   return result.Pass();
 }
 
-// static
-scoped_ptr<AuthenticatorFactory>
-    Me2MeHostAuthenticatorFactory::CreateRejecting() {
-  return make_scoped_ptr(new Me2MeHostAuthenticatorFactory());
-}
-
 Me2MeHostAuthenticatorFactory::Me2MeHostAuthenticatorFactory() {
 }
 

remoting/protocol/me2me_host_authenticator_factory.h

@@ -43,10 +43,6 @@ class Me2MeHostAuthenticatorFactory : public AuthenticatorFactory {
       scoped_refptr<RsaKeyPair> key_pair,
       scoped_ptr<TokenValidatorFactory> token_validator_factory);
 
-  // Create a factory that dispenses rejecting authenticators (used when the
-  // host config/policy is inconsistent)
-  static scoped_ptr<AuthenticatorFactory> CreateRejecting();
-
   Me2MeHostAuthenticatorFactory();
   ~Me2MeHostAuthenticatorFactory() override;
 

remoting/protocol/network_settings.h

@@ -9,6 +9,7 @@
 
 #include "base/basictypes.h"
 #include "base/logging.h"
+#include "remoting/protocol/port_range.h"
 
 namespace remoting {
 namespace protocol {
@@ -42,32 +43,17 @@ struct NetworkSettings {
         NAT_TRAVERSAL_OUTGOING
   };
 
-  NetworkSettings()
-      : flags(NAT_TRAVERSAL_DISABLED),
-        min_port(0),
-        max_port(0) {
+  NetworkSettings() : flags(NAT_TRAVERSAL_DISABLED) {
     DCHECK(!(flags & (NAT_TRAVERSAL_STUN | NAT_TRAVERSAL_RELAY)) ||
            (flags & NAT_TRAVERSAL_OUTGOING));
   }
 
-  explicit NetworkSettings(uint32 flags)
-      : flags(flags),
-        min_port(0),
-        max_port(0) {
-  }
-
-  // Parse string in the form "<min_port>-<max_port>". E.g. "12400-12409".
-  // Returns true if string was parsed successfuly.
-  static bool ParsePortRange(const std::string& port_range,
-                             uint16* out_min_port,
-                             uint16* out_max_port);
+  explicit NetworkSettings(uint32 flags) : flags(flags) {}
 
   uint32 flags;
 
-  // |min_port| and |max_port| specify range (inclusive) of ports used by
-  // P2P sessions. Any port can be used when both values are set to 0.
-  uint16 min_port;
-  uint16 max_port;
+  // Range of ports used by P2P sessions.
+  PortRange port_range;
 };
 
 }  // namespace protocol

remoting/protocol/network_settings_unittest.cc

-// Copyright 2014 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/network_settings.h"
-
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace remoting {
-namespace protocol {
-
-TEST(ParsePortRange, Basic) {
-  uint16 min, max;
-
-  // Valid range
-  EXPECT_TRUE(NetworkSettings::ParsePortRange("1-65535", &min, &max));
-  EXPECT_EQ(1u, min);
-  EXPECT_EQ(65535u, max);
-
-  EXPECT_TRUE(NetworkSettings::ParsePortRange(" 1 - 65535 ", &min, &max));
-  EXPECT_EQ(1u, min);
-  EXPECT_EQ(65535u, max);
-
-  EXPECT_TRUE(NetworkSettings::ParsePortRange("12400-12400", &min, &max));
-  EXPECT_EQ(12400u, min);
-  EXPECT_EQ(12400u, max);
-
-  // Invalid
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("", &min, &max));
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("-65535", &min, &max));
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("1-", &min, &max));
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("-", &min, &max));
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("-1-65535", &min, &max));
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("1--65535", &min, &max));
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("1-65535-", &min, &max));
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("0-65535", &min, &max));
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("1-65536", &min, &max));
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("1-4294967295", &min, &max));
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("10-1", &min, &max));
-  EXPECT_FALSE(NetworkSettings::ParsePortRange("1foo-2bar", &min, &max));
-}
-
-}  // namespace protocol
-}  // namespace remoting

remoting/protocol/network_settings.cc → remoting/protocol/port_range.cc

-// Copyright 2014 The Chromium Authors. All rights reserved.
+// Copyright 2015 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/network_settings.h"
+#include "remoting/protocol/port_range.h"
 
 #include <limits.h>
 #include <stdlib.h>
@@ -11,23 +11,25 @@
 #include "base/strings/string_util.h"
 
 namespace remoting {
-namespace protocol {
 
-// static
-  bool NetworkSettings::ParsePortRange(const std::string& port_range,
-                                       uint16* out_min_port,
-                                       uint16* out_max_port) {
+bool PortRange::Parse(const std::string& port_range, PortRange* result) {
+  DCHECK(result);
+
+  if (port_range.empty()) {
+    result->min_port = 0;
+    result->max_port = 0;
+    return true;
+  }
+
   size_t separator_index = port_range.find('-');
   if (separator_index == std::string::npos)
     return false;
 
   std::string min_port_string, max_port_string;
   base::TrimWhitespaceASCII(port_range.substr(0, separator_index),
-                            base::TRIM_ALL,
-                            &min_port_string);
+                            base::TRIM_ALL, &min_port_string);
   base::TrimWhitespaceASCII(port_range.substr(separator_index + 1),
-                            base::TRIM_ALL,
-                            &max_port_string);
+                            base::TRIM_ALL, &max_port_string);
 
   unsigned min_port, max_port;
   if (!base::StringToUint(min_port_string, &min_port) ||
@@ -38,10 +40,18 @@ namespace protocol {
   if (min_port == 0 || min_port > max_port || max_port > USHRT_MAX)
     return false;
 
-  *out_min_port = static_cast<uint16>(min_port);
-  *out_max_port = static_cast<uint16>(max_port);
+  result->min_port = static_cast<uint16>(min_port);
+  result->max_port = static_cast<uint16>(max_port);
   return true;
 }
 
-}  // namespace protocol
+std::ostream& operator<<(std::ostream& os, const PortRange& port_range) {
+  if (port_range.is_null()) {
+    os << "<no port range specified>";
+  } else {
+    os << "[" << port_range.min_port << ", " << port_range.max_port << "]";
+  }
+  return os;
+}
+
 }  // namespace remoting

remoting/protocol/port_range.h

+// Copyright 2015 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.
+
+#ifndef REMOTING_PROTOCOL_PORT_RANGE_H_
+#define REMOTING_PROTOCOL_PORT_RANGE_H_
+
+#include <ostream>
+#include <string>
+
+#include "base/basictypes.h"
+
+namespace remoting {
+
+// Wrapper for a value of UdpPortRange policy.
+struct PortRange {
+  // Both |min_port| and |max_port| are inclusive.
+  uint16 min_port;
+  uint16 max_port;
+
+  // Returns true if |port_range| passed to Parse was an empty string
+  // (or if |this| has been initialized by the default constructor below).
+  inline bool is_null() const { return (min_port == 0) && (max_port == 0); }
+
+  // Parse string in the form "<min_port>-<max_port>". E.g. "12400-12409".
+  // Returns true if string was parsed successfuly.
+  //
+  // Returns false and doesn't modify |result| if parsing fails (i.e. when
+  // |port_range| doesn't represent a valid port range).
+  static bool Parse(const std::string& port_range, PortRange* result);
+
+  PortRange() : min_port(0), max_port(0) {}
+};
+
+std::ostream& operator<<(std::ostream& os, const PortRange& port_range);
+
+}  // namespace remoting
+
+#endif  // REMOTING_PROTOCOL_PORT_RANGE_H_

remoting/protocol/port_range_unittest.cc

+// Copyright 2015 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/port_range.h"
+
+#include <sstream>
+
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace remoting {
+
+TEST(PortRange, ParseEmpty) {
+  PortRange port_range;
+
+  EXPECT_TRUE(PortRange::Parse("", &port_range));
+  EXPECT_TRUE(port_range.is_null());
+}
+
+TEST(PortRange, ParseValid) {
+  PortRange port_range;
+
+  EXPECT_TRUE(PortRange::Parse("1-65535", &port_range));
+  EXPECT_FALSE(port_range.is_null());
+  EXPECT_EQ(1u, port_range.min_port);
+  EXPECT_EQ(65535u, port_range.max_port);
+
+  EXPECT_TRUE(PortRange::Parse(" 1 - 65535 ", &port_range));
+  EXPECT_FALSE(port_range.is_null());
+  EXPECT_EQ(1u, port_range.min_port);
+  EXPECT_EQ(65535u, port_range.max_port);
+
+  EXPECT_TRUE(PortRange::Parse("12400-12400", &port_range));
+  EXPECT_FALSE(port_range.is_null());
+  EXPECT_EQ(12400u, port_range.min_port);
+  EXPECT_EQ(12400u, port_range.max_port);
+}
+
+TEST(PortRange, ParseInvalid) {
+  PortRange port_range;
+  port_range.min_port = 123;
+  port_range.max_port = 456;
+
+  EXPECT_FALSE(PortRange::Parse("-65535", &port_range));
+  EXPECT_FALSE(PortRange::Parse("1-", &port_range));
+  EXPECT_FALSE(PortRange::Parse("-", &port_range));
+  EXPECT_FALSE(PortRange::Parse("-1-65535", &port_range));
+  EXPECT_FALSE(PortRange::Parse("1--65535", &port_range));
+  EXPECT_FALSE(PortRange::Parse("1-65535-", &port_range));
+  EXPECT_FALSE(PortRange::Parse("0-65535", &port_range));
+  EXPECT_FALSE(PortRange::Parse("1-65536", &port_range));
+  EXPECT_FALSE(PortRange::Parse("1-4294967295", &port_range));
+  EXPECT_FALSE(PortRange::Parse("10-1", &port_range));
+  EXPECT_FALSE(PortRange::Parse("1foo-2bar", &port_range));
+
+  // Unsuccessful parses should NOT modify their output.
+  EXPECT_EQ(123, port_range.min_port);
+  EXPECT_EQ(456, port_range.max_port);
+}
+
+TEST(PortRange, Output) {
+  PortRange port_range;
+  port_range.min_port = 123;
+  port_range.max_port = 456;
+
+  std::ostringstream str;
+  str << port_range;
+
+  EXPECT_THAT(str.str(), testing::MatchesRegex(".*123.*456.*"));
+}
+
+}  // namespace remoting

remoting/remoting_host_srcs.gypi

@@ -218,6 +218,8 @@
       'host/single_window_input_injector_linux.cc',
       'host/single_window_input_injector_mac.cc',
       'host/single_window_input_injector_win.cc',
+      'host/third_party_auth_config.cc',
+      'host/third_party_auth_config.h',
       'host/token_validator_base.cc',
       'host/token_validator_base.h',
       'host/token_validator_factory_impl.cc',

remoting/remoting_srcs.gypi

@@ -152,7 +152,6 @@
       'protocol/negotiating_client_authenticator.h',
       'protocol/negotiating_host_authenticator.cc',
       'protocol/negotiating_host_authenticator.h',
-      'protocol/network_settings.cc',
       'protocol/network_settings.h',
       'protocol/pairing_authenticator_base.cc',
       'protocol/pairing_authenticator_base.h',
@@ -162,6 +161,8 @@
       'protocol/pairing_host_authenticator.h',
       'protocol/pairing_registry.cc',
       'protocol/pairing_registry.h',
+      'protocol/port_range.cc',
+      'protocol/port_range.h',
       'protocol/pseudotcp_channel_factory.cc',
       'protocol/pseudotcp_channel_factory.h',
       'protocol/secure_channel_factory.cc',

remoting/remoting_test.gypi

@@ -187,6 +187,7 @@
         'host/setup/oauth_helper_unittest.cc',
         'host/setup/pin_validator_unittest.cc',
         'host/shaped_desktop_capturer_unittest.cc',
+        'host/third_party_auth_config_unittest.cc',
         'host/token_validator_factory_impl_unittest.cc',
         'host/video_frame_pump_unittest.cc',
         'host/video_frame_recorder_unittest.cc',
@@ -214,8 +215,8 @@
         'protocol/monitored_video_stub_unittest.cc',
         'protocol/mouse_input_filter_unittest.cc',
         'protocol/negotiating_authenticator_unittest.cc',
-        'protocol/network_settings_unittest.cc',
         'protocol/pairing_registry_unittest.cc',
+        'protocol/port_range_unittest.cc',
         'protocol/ppapi_module_stub.cc',
         'protocol/ssl_hmac_channel_authenticator_unittest.cc',
         'protocol/third_party_authenticator_unittest.cc',