ONC: Add IPAddressConfigType and NameServersConfigType

This CL introdcues IPAddressConfigType and NameServersConfigType If either of these is set to 'Static', a 'StaticIPConfig' object must be provided and must specify any non-default configuration. If neither is set to 'Static', the StaticIPConfig object will be ignored. Originally uploaded as https://codereview.chromium.org/750313003/ BUG=411289 Review URL: https://codereview.chromium.org/749013003 Cr-Commit-Position: refs/heads/master@{#310325}
FairyWorld · Jan 7, 2015 · 75a3c1d · 75a3c1d
1 parent 2153328
commit 75a3c1d
Show file tree

Hide file tree

Showing 21 changed files with 206 additions and 9 deletions.
diff --git a/chromeos/network/onc/onc_merger.cc b/chromeos/network/onc/onc_merger.cc
@@ -369,7 +369,9 @@ class MergeToAugmented : public MergeToEffective {
       // This field is not part of the provided ONCSignature, thus it cannot be
       // controlled by policy. Return the plain active value instead of an
       // augmented dictionary.
-      return make_scoped_ptr(values.active_setting->DeepCopy());
+      if (values.active_setting)
+        return make_scoped_ptr(values.active_setting->DeepCopy());
+      return nullptr;
     }
 
     // This field is part of the provided ONCSignature, thus it can be

diff --git a/chromeos/network/onc/onc_normalizer.cc b/chromeos/network/onc/onc_normalizer.cc
@@ -173,6 +173,19 @@ void Normalizer::NormalizeNetworkConfiguration(base::DictionaryValue* network) {
   RemoveEntryUnless(network,
                     ::onc::network_config::kWiFi,
                     type == ::onc::network_type::kWiFi);
+
+  std::string ip_address_config_type, name_servers_config_type;
+  network->GetStringWithoutPathExpansion(
+      ::onc::network_config::kIPAddressConfigType, &ip_address_config_type);
+  network->GetStringWithoutPathExpansion(
+      ::onc::network_config::kNameServersConfigType, &name_servers_config_type);
+  RemoveEntryUnless(
+      network, ::onc::network_config::kStaticIPConfig,
+      (ip_address_config_type == ::onc::network_config::kIPConfigTypeStatic) ||
+          (name_servers_config_type ==
+           ::onc::network_config::kIPConfigTypeStatic));
+  // TODO(pneubeck): Remove fields from StaticIPConfig not specified by
+  // IP[Address|Nameservers]ConfigType.
 }
 
 void Normalizer::NormalizeOpenVPN(base::DictionaryValue* openvpn) {

diff --git a/chromeos/network/onc/onc_normalizer_unittest.cc b/chromeos/network/onc/onc_normalizer_unittest.cc
@@ -13,6 +13,23 @@
 namespace chromeos {
 namespace onc {
 
+// Validate that an irrelevant StaticIPConfig dictionary will be removed.
+TEST(ONCNormalizerTest, RemoveStaticIPConfig) {
+  Normalizer normalizer(true);
+  scoped_ptr<const base::DictionaryValue> data(
+      test_utils::ReadTestDictionary("settings_with_normalization.json"));
+
+  const base::DictionaryValue* original = NULL;
+  const base::DictionaryValue* expected_normalized = NULL;
+  data->GetDictionary("irrelevant-staticipconfig", &original);
+  data->GetDictionary("irrelevant-staticipconfig-normalized",
+                      &expected_normalized);
+
+  scoped_ptr<base::DictionaryValue> actual_normalized =
+      normalizer.NormalizeObject(&kNetworkConfigurationSignature, *original);
+  EXPECT_TRUE(test_utils::Equals(expected_normalized, actual_normalized.get()));
+}
+
 // This test case is about validating valid ONC objects.
 TEST(ONCNormalizerTest, NormalizeNetworkConfigurationEthernetAndVPN) {
   Normalizer normalizer(true);

diff --git a/chromeos/network/onc/onc_signature.cc b/chromeos/network/onc/onc_signature.cc
@@ -301,7 +301,9 @@ const OncFieldSignature network_configuration_fields[] = {
     { ::onc::network_config::kCellular, &kCellularSignature},
     { ::onc::network_config::kEthernet, &kEthernetSignature},
     { ::onc::network_config::kGUID, &kStringSignature},
+    { ::onc::network_config::kIPAddressConfigType, &kStringSignature},
     { ::onc::network_config::kName, &kStringSignature},
+    { ::onc::network_config::kNameServersConfigType, &kStringSignature},
     { ::onc::network_config::kPriority, &kIntegerSignature},
     { ::onc::network_config::kProxySettings, &kProxySettingsSignature},
     { ::onc::kRecommended, &kRecommendedSignature},

diff --git a/chromeos/network/onc/onc_translator_onc_to_shill.cc b/chromeos/network/onc/onc_translator_onc_to_shill.cc
@@ -257,6 +257,19 @@ void LocalTranslator::TranslateNetworkConfiguration() {
   if (type == ::onc::network_type::kVPN)
     CopyFieldFromONCToShill(::onc::network_config::kName, shill::kNameProperty);
 
+  std::string ip_address_config_type, name_servers_config_type;
+  onc_object_->GetStringWithoutPathExpansion(
+      ::onc::network_config::kIPAddressConfigType, &ip_address_config_type);
+  onc_object_->GetStringWithoutPathExpansion(
+      ::onc::network_config::kNameServersConfigType, &name_servers_config_type);
+  if ((ip_address_config_type == ::onc::network_config::kIPConfigTypeDHCP) ||
+      (name_servers_config_type == ::onc::network_config::kIPConfigTypeDHCP)) {
+    // If either type is set to DHCP, provide an empty dictionary to ensure
+    // that any unset properties are cleared. Note: if either type is specified,
+    // the other type defaults to DHCP if not specified.
+    shill_dictionary_->SetWithoutPathExpansion(shill::kStaticIPConfigProperty,
+                                               new base::DictionaryValue);
+  }
   CopyFieldsAccordingToSignature();
 }
 

diff --git a/chromeos/network/onc/onc_translator_shill_to_onc.cc b/chromeos/network/onc/onc_translator_shill_to_onc.cc
@@ -486,11 +486,30 @@ void ShillToONCTranslator::TranslateNetworkWithState() {
                                 *saved_ipconfig);
   }
 
+  // Translate the StaticIPConfig object and set the IP config types.
   const base::DictionaryValue* static_ipconfig = nullptr;
   if (shill_dictionary_->GetDictionaryWithoutPathExpansion(
-        shill::kStaticIPConfigProperty, &static_ipconfig)) {
-    TranslateAndAddNestedObject(::onc::network_config::kStaticIPConfig,
-                                *static_ipconfig);
+          shill::kStaticIPConfigProperty, &static_ipconfig)) {
+    std::string ip_address;
+    if (static_ipconfig->GetStringWithoutPathExpansion(shill::kAddressProperty,
+                                                       &ip_address) &&
+        !ip_address.empty()) {
+      onc_object_->SetStringWithoutPathExpansion(
+          ::onc::network_config::kIPAddressConfigType,
+          ::onc::network_config::kIPConfigTypeStatic);
+    }
+    const base::ListValue* name_servers = nullptr;
+    if (static_ipconfig->GetListWithoutPathExpansion(
+            shill::kNameServersProperty, &name_servers) &&
+        !name_servers->empty()) {
+      onc_object_->SetStringWithoutPathExpansion(
+          ::onc::network_config::kNameServersConfigType,
+          ::onc::network_config::kIPConfigTypeStatic);
+    }
+    if (!ip_address.empty() || (name_servers && !name_servers->empty())) {
+      TranslateAndAddNestedObject(::onc::network_config::kStaticIPConfig,
+                                  *static_ipconfig);
+    }
   }
 }
 

diff --git a/chromeos/network/onc/onc_translator_unittest.cc b/chromeos/network/onc/onc_translator_unittest.cc
@@ -50,6 +50,7 @@ INSTANTIATE_TEST_CASE_P(
                        "shill_wifi_clientcert.json"),
         std::make_pair("valid_wifi_clientref.onc", "shill_wifi_clientref.json"),
         std::make_pair("valid_l2tpipsec.onc", "shill_l2tpipsec.json"),
+        std::make_pair("wifi_dhcp.onc", "shill_wifi_dhcp.json"),
         std::make_pair("l2tpipsec_clientcert_with_cert_pems.onc",
                        "shill_l2tpipsec_clientcert.json"),
         std::make_pair("valid_openvpn_with_cert_pems.onc",

diff --git a/chromeos/network/onc/onc_validator.cc b/chromeos/network/onc/onc_validator.cc
@@ -522,7 +522,15 @@ bool Validator::ValidateNetworkConfiguration(base::DictionaryValue* result) {
       ::onc::network_type::kEthernet, ::onc::network_type::kVPN,
       ::onc::network_type::kWiFi, ::onc::network_type::kCellular};
   const std::vector<const char*> valid_types(toVector(kValidTypes));
+  const char* const kValidIPConfigTypes[] = {kIPConfigTypeDHCP,
+                                             kIPConfigTypeStatic};
+  const std::vector<const char*> valid_ipconfig_types(
+      toVector(kValidIPConfigTypes));
   if (FieldExistsAndHasNoValidValue(*result, kType, valid_types) ||
+      FieldExistsAndHasNoValidValue(*result, kIPAddressConfigType,
+                                    valid_ipconfig_types) ||
+      FieldExistsAndHasNoValidValue(*result, kNameServersConfigType,
+                                    valid_ipconfig_types) ||
       FieldExistsAndIsEmpty(*result, kGUID)) {
     return false;
   }
@@ -538,6 +546,18 @@ bool Validator::ValidateNetworkConfiguration(base::DictionaryValue* result) {
     all_required_exist &=
         RequireField(*result, kName) && RequireField(*result, kType);
 
+    std::string ip_address_config_type, name_servers_config_type;
+    result->GetStringWithoutPathExpansion(kIPAddressConfigType,
+                                          &ip_address_config_type);
+    result->GetStringWithoutPathExpansion(kNameServersConfigType,
+                                          &name_servers_config_type);
+    if (ip_address_config_type == kIPConfigTypeStatic ||
+        name_servers_config_type == kIPConfigTypeStatic) {
+      // TODO(pneubeck): Add ValidateStaticIPConfig and confirm that the
+      // correct properties are provided based on the config type.
+      all_required_exist &= RequireField(*result, kStaticIPConfig);
+    }
+
     std::string type;
     result->GetStringWithoutPathExpansion(kType, &type);
 

diff --git a/chromeos/test/data/network/augmented_merge.json b/chromeos/test/data/network/augmented_merge.json
@@ -13,7 +13,12 @@
       "IPAddress": "124.124.124.124",
       "NameServers": [ "1.1.1.5", "1.1.1.6" ],
       "RoutingPrefix": 25,
-      "Type": "IPv4",
+      "Type": "IPv4"
+   },
+   "IPAddressConfigType": {
+     "DevicePolicy": "Static",
+     "Effective": "UserPolicy",
+     "UserSetting": "Static"
    },
    "StaticIPConfig": {
       "IPAddress": {

diff --git a/chromeos/test/data/network/device_policy.onc b/chromeos/test/data/network/device_policy.onc
@@ -1,6 +1,7 @@
 { "GUID": "123",
   "Type": "VPN",
   "Name": "testopenvpn",
+  "IPAddressConfigType": "Static",
   "StaticIPConfig": {
     "Type": "IPv4",
     "IPAddress": "127.0.0.1"

diff --git a/chromeos/test/data/network/managed_toplevel1.onc b/chromeos/test/data/network/managed_toplevel1.onc
@@ -3,6 +3,8 @@
   [ { "GUID": "123",
       "Type": "VPN",
       "Name": "testopenvpn",
+      "IPAddressConfigType": "Static",
+      "NameServersConfigType": "Static",
       "StaticIPConfig": {
           "Gateway":"1.1.1.1",
           "IPAddress": "127.0.0.1",

diff --git a/chromeos/test/data/network/settings_with_normalization.json b/chromeos/test/data/network/settings_with_normalization.json
@@ -1,4 +1,24 @@
 {
+    "irrelevant-staticipconfig": {
+        "GUID": "guid",
+        "Type": "Ethernet",
+        "Name": "name",
+        "Ethernet": {
+            "Authentication": "None"
+        },
+        "StaticIPConfig": {
+            "Gateway":"1.1.1.1",
+            "IPAddress": "127.0.0.1"
+        }
+    },
+    "irrelevant-staticipconfig-normalized": {
+        "GUID": "guid",
+        "Type": "Ethernet",
+        "Name": "name",
+        "Ethernet": {
+            "Authentication": "None"
+        }
+    },
     "ethernet-and-vpn": {
         "Recommended": [],
         "GUID": "guid",

diff --git a/chromeos/test/data/network/shill_wifi_dhcp.json b/chromeos/test/data/network/shill_wifi_dhcp.json
@@ -0,0 +1,10 @@
+{
+   "AutoConnect": true,
+   "GUID": "{b3e26c32-4289-41ea-b2a4-d0554acb1f67}",
+   "Mode": "managed",
+   "StaticIPConfig": {},
+   "Passphrase": "some passphrase",
+   "Security": "psk",
+   "Type": "wifi",
+   "WiFi.HexSSID": "576966695769746844484350", // "WifiWithDHCP"
+}
diff --git a/chromeos/test/data/network/toplevel_wifi_open.onc b/chromeos/test/data/network/toplevel_wifi_open.onc
@@ -3,6 +3,7 @@
   "NetworkConfigurations": [
     {
       "GUID": "{485d6076-dd44-6b6d-69787465725f5040}",
+      "IPAddressConfigType": "DHCP",
       "Type": "WiFi",
       "Name": "My WiFi Network",
       "WiFi": {

diff --git a/chromeos/test/data/network/translation_of_shill_ethernet.onc b/chromeos/test/data/network/translation_of_shill_ethernet.onc
@@ -3,7 +3,9 @@
     "Authentication":"None"
   },
   "GUID":"guid",
+  "IPAddressConfigType":"Static",
   "Name":"",
+  "NameServersConfigType":"Static",
   "StaticIPConfig":{
     "Gateway":"1.1.1.7",
     "IPAddress":"125.125.125.125",

diff --git a/chromeos/test/data/network/translation_of_shill_ethernet_with_ipconfig.onc b/chromeos/test/data/network/translation_of_shill_ethernet_with_ipconfig.onc
@@ -3,6 +3,7 @@
     "Authentication":"None"
   },
   "GUID":"guid",
+  "IPAddressConfigType":"Static",
   "IPConfigs":[
     {
       "Gateway":"1.1.1.1",
@@ -24,6 +25,7 @@
     }
   ],
   "Name":"",
+  "NameServersConfigType":"Static",
   "SavedIPConfig":{
     "Gateway":"1.1.1.4",
     "IPAddress":"124.124.124.124",

diff --git a/chromeos/test/data/network/user.onc b/chromeos/test/data/network/user.onc
@@ -11,6 +11,7 @@
   "ProxySettings": {
     "Type": "Direct"
   },
+  "IPAddressConfigType": "Static",
   "StaticIPConfig": {
     "Type": "IPv4",
     "IPAddress": "1.2.3.4",

diff --git a/chromeos/test/data/network/wifi_dhcp.onc b/chromeos/test/data/network/wifi_dhcp.onc
@@ -0,0 +1,12 @@
+{
+    "GUID": "{b3e26c32-4289-41ea-b2a4-d0554acb1f67}",
+    "IPAddressConfigType": "DHCP",
+    "Name": "Wifi With DHCP",
+    "Type": "WiFi",
+    "WiFi": {
+        "AutoConnect": true,
+        "Passphrase": "some passphrase",
+        "SSID": "WifiWithDHCP",
+        "Security": "WPA-PSK"
+    }
+}
diff --git a/components/onc/docs/onc_spec.html b/components/onc/docs/onc_spec.html
@@ -251,6 +251,44 @@ <h1>Network Configuration</h1>
       string.
     </dd>
 
+    <dt class="field">IPAddressConfigType</dt>
+    <dd>
+      <span class="field_meta">
+        (optional if <span class="field">Remove</span> is
+        <span class="value">false</span>, otherwise ignored. Defaults to
+        <span class="value">DHCP</span> if
+        <span class="field">NameServersConfigType</span> is specified)
+        <span class="type">string</span>
+      </span>
+      <span class="rule">
+        <span class="rule_id"></span>
+        Allowed values are <span class="value">DHCP</span> and
+        <span class="value">Static</span>.
+      </span>
+      Determines whether the IP Address configuration is statically configured,
+      see <span class="field">StaticIPConfig</span>, or automatically configured
+      using DHCP.
+    </dd>
+
+    <dt class="field">NameServersConfigType</dt>
+    <dd>
+      <span class="field_meta">
+        (optional if <span class="field">Remove</span> is
+        <span class="value">false</span>, otherwise ignored. Defaults to
+        <span class="value">DHCP</span> if
+        <span class="field">IPAddressConfigType</span> is specified)
+        <span class="type">string</span>
+      </span>
+      <span class="rule">
+        <span class="rule_id"></span>
+        Allowed values are <span class="value">DHCP</span> and
+        <span class="value">Static</span>.
+      </span>
+      Determines whether the NameServers configuration is statically configured,
+      see <span class="field">StaticIPConfig</span>, or automatically configured
+      using DHCP.
+    </dd>
+
     <dt class="field">IPConfigs</dt>
     <dd>
       <span class="field_meta">
@@ -263,12 +301,19 @@ <h1>Network Configuration</h1>
     <dt class="field">StaticIPConfig</dt>
     <dd>
       <span class="field_meta">
-        (optional if <span class="field">Remove</span> is
-        <span class="value">false</span>, otherwise ignored)
+        (required if <span class="field">IPAddressConfigType</span> or
+        <span class="field">NameServersConfigType</span> is set to
+        <span class="value">Static</span>)
         <span class="type">IPConfig</span>
       </span>
       Each property set in this IPConfig object overrides the respective
       parameter received over DHCP.
+      If <span class="field">IPAddressConfigType</span> is set to
+      <span class="value">Static</span>, <span class="field">IPAddress</span>
+      and <span class="field">Gateway</span> are required.
+      If <span class="field">NameServersConfigType</span> is set to
+      <span class="value">Static</span>, <span class="field">NameServers</span>
+      is required.
     </dd>
 
     <dt class="field">SavedIPConfig</dt>
@@ -533,7 +578,7 @@ <h1>IPConfig</h1>
     <dt class="field">IPAddress</dt>
     <dd>
       <span class="field_meta">
-        (required)
+        (optional)
         <span class="type">string</span>
       </span>
       Describes the IPv4 or IPv6 address of a connection, depending on the value
@@ -559,7 +604,8 @@ <h1>IPConfig</h1>
     <dt class="field">Gateway</dt>
     <dd>
       <span class="field_meta">
-        (optional)
+        (required if <span class="field">IPAddress</span> is set. Otherwise
+        ignored.)
         <span class="type">string</span>
       </span>
       Describes the gateway address to use for the configuration. Must match

diff --git a/components/onc/onc_constants.cc b/components/onc/onc_constants.cc
@@ -39,8 +39,12 @@ const char kDevice[] = "Device";
 const char kErrorState[] = "ErrorState";
 const char kEthernet[] = "Ethernet";
 const char kGUID[] = "GUID";
+const char kIPAddressConfigType[] = "IPAddressConfigType";
 const char kIPConfigs[] = "IPConfigs";
+const char kIPConfigTypeDHCP[] = "DHCP";
+const char kIPConfigTypeStatic[] = "Static";
 const char kMacAddress[] = "MacAddress";
+const char kNameServersConfigType[] = "NameServersConfigType";
 const char kName[] = "Name";
 const char kPriority[] = "Priority";
 const char kProxySettings[] = "ProxySettings";

diff --git a/components/onc/onc_constants.h b/components/onc/onc_constants.h
@@ -61,10 +61,14 @@ ONC_EXPORT extern const char kCellular[];
 ONC_EXPORT extern const char kDevice[];
 ONC_EXPORT extern const char kEthernet[];
 ONC_EXPORT extern const char kGUID[];
+ONC_EXPORT extern const char kIPAddressConfigType[];
 ONC_EXPORT extern const char kIPConfigs[];
+ONC_EXPORT extern const char kIPConfigTypeDHCP[];
+ONC_EXPORT extern const char kIPConfigTypeStatic[];
 ONC_EXPORT extern const char kSavedIPConfig[];
 ONC_EXPORT extern const char kStaticIPConfig[];
 ONC_EXPORT extern const char kMacAddress[];
+ONC_EXPORT extern const char kNameServersConfigType[];
 ONC_EXPORT extern const char kName[];
 ONC_EXPORT extern const char kPriority[];
 ONC_EXPORT extern const char kProxySettings[];