Commit e9d087a0 authored by Zakhar Voit's avatar Zakhar Voit Committed by Commit Bot

Allow user to modify network settings not enforced by device policy.

* ONC normalizer now removes StaticIPConfig fields that are
overriden by IPAddressConfigType and NameServersConfigType.

* ONC validator validates StaticIPConfig properly (e.g. requires
fields based on IPAddressConfigType and NameServersConfigType)

Bug: 847429
Change-Id: I47fceaaf10f41346afba552a35206fcdb130a887
Reviewed-on: https://chromium-review.googlesource.com/c/1288576Reviewed-by: default avatarAlexander Hendrich <hendrich@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Commit-Queue: Zakhar Voit <voit@google.com>
Cr-Commit-Position: refs/heads/master@{#613409}
parent 6aa20f01
......@@ -419,6 +419,8 @@ class NetworkingPrivateChromeOSApiTest : public extensions::ExtensionApiTest {
base::Value(shill::kTetheringNotDetectedState));
base::DictionaryValue static_ipconfig;
static_ipconfig.SetKey(shill::kAddressProperty, base::Value("1.2.3.4"));
static_ipconfig.SetKey(shill::kGatewayProperty, base::Value("0.0.0.0"));
static_ipconfig.SetKey(shill::kPrefixlenProperty, base::Value(1));
service_test_->SetServiceProperty(
kWifi1ServicePath, shill::kStaticIPConfigProperty, static_ipconfig);
base::ListValue frequencies1;
......
......@@ -560,6 +560,8 @@ var availableTests = [
Source: 'User',
StaticIPConfig: {
IPAddress: '1.2.3.4',
Gateway: '0.0.0.0',
RoutingPrefix: 1,
Type: 'IPv4'
},
Type: NetworkType.WI_FI,
......@@ -749,7 +751,9 @@ var availableTests = [
},
IPAddressConfigType: 'Static',
StaticIPConfig: {
IPAddress: '1.2.3.4'
IPAddress: '1.2.3.4',
Gateway: '0.0.0.0',
RoutingPrefix: 1
}
};
chrome.networkingPrivate.setProperties(
......@@ -769,6 +773,8 @@ var availableTests = [
assertTrue('StaticIPConfig' in result);
assertEq('1.2.3.4',
result['StaticIPConfig']['IPAddress']);
assertEq('0.0.0.0', result['StaticIPConfig']['Gateway']);
assertEq(1, result['StaticIPConfig']['RoutingPrefix']);
}));
}));
}));
......
......@@ -317,7 +317,9 @@ var availableTests = [
},
IPAddressConfigType: 'Static',
StaticIPConfig: {
IPAddress: '1.2.3.4'
IPAddress: '1.2.3.4',
Gateway: '0.0.0.0',
RoutingPrefix: 1
}
};
chrome.networkingPrivate.setProperties(
......@@ -337,6 +339,8 @@ var availableTests = [
assertTrue('StaticIPConfig' in result);
assertEq('1.2.3.4',
result['StaticIPConfig']['IPAddress']);
assertEq('0.0.0.0', result['StaticIPConfig']['Gateway']);
assertEq(1, result['StaticIPConfig']['RoutingPrefix']);
done();
}));
}));
......
......@@ -70,15 +70,26 @@ std::unique_ptr<base::DictionaryValue> Normalizer::MapObject(
namespace {
void RemoveEntryUnless(base::DictionaryValue* dict,
void RemoveEntryUnless(base::Value* dict,
const std::string& path,
bool condition) {
DCHECK(dict->is_dict());
if (!condition && dict->FindKey(path)) {
NET_LOG(ERROR) << "onc::Normalizer:Removing: " << path;
dict->RemoveKey(path);
}
}
bool IsIpConfigTypeStatic(base::Value* network,
const std::string& ip_config_type_key) {
DCHECK(network->is_dict());
base::Value* ip_config_type =
network->FindKeyOfType(ip_config_type_key, base::Value::Type::STRING);
return ip_config_type && ip_config_type->GetString() ==
::onc::network_config::kIPConfigTypeStatic;
}
} // namespace
void Normalizer::NormalizeCertificate(base::DictionaryValue* cert) {
......@@ -172,18 +183,7 @@ void Normalizer::NormalizeNetworkConfiguration(base::DictionaryValue* 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.
NormalizeStaticIPConfigForNetwork(network);
}
void Normalizer::NormalizeOpenVPN(base::DictionaryValue* openvpn) {
......@@ -248,5 +248,48 @@ void Normalizer::NormalizeWiFi(base::DictionaryValue* wifi) {
FillInHexSSIDField(wifi);
}
void Normalizer::NormalizeStaticIPConfigForNetwork(
base::DictionaryValue* network) {
const bool ip_config_type_is_static = IsIpConfigTypeStatic(
network, ::onc::network_config::kIPAddressConfigType);
const bool name_servers_type_is_static = IsIpConfigTypeStatic(
network, ::onc::network_config::kNameServersConfigType);
RemoveEntryUnless(network, ::onc::network_config::kStaticIPConfig,
ip_config_type_is_static || name_servers_type_is_static);
base::Value* static_ip_config = network->FindKeyOfType(
::onc::network_config::kStaticIPConfig, base::Value::Type::DICTIONARY);
bool all_ip_fields_exist = false;
bool name_servers_exist = false;
if (static_ip_config) {
all_ip_fields_exist =
static_ip_config->FindKey(::onc::ipconfig::kIPAddress) &&
static_ip_config->FindKey(::onc::ipconfig::kGateway) &&
static_ip_config->FindKey(::onc::ipconfig::kRoutingPrefix);
name_servers_exist =
static_ip_config->FindKey(::onc::ipconfig::kNameServers);
RemoveEntryUnless(static_ip_config, ::onc::ipconfig::kIPAddress,
all_ip_fields_exist && ip_config_type_is_static);
RemoveEntryUnless(static_ip_config, ::onc::ipconfig::kGateway,
all_ip_fields_exist && ip_config_type_is_static);
RemoveEntryUnless(static_ip_config, ::onc::ipconfig::kRoutingPrefix,
all_ip_fields_exist && ip_config_type_is_static);
RemoveEntryUnless(static_ip_config, ::onc::ipconfig::kNameServers,
name_servers_type_is_static);
RemoveEntryUnless(network, ::onc::network_config::kStaticIPConfig,
!static_ip_config->DictEmpty());
}
RemoveEntryUnless(network, ::onc::network_config::kIPAddressConfigType,
!ip_config_type_is_static || all_ip_fields_exist);
RemoveEntryUnless(network, ::onc::network_config::kNameServersConfigType,
!name_servers_type_is_static || name_servers_exist);
}
} // namespace onc
} // namespace chromeos
......@@ -50,6 +50,7 @@ class CHROMEOS_EXPORT Normalizer : public Mapper {
void NormalizeProxySettings(base::DictionaryValue* proxy);
void NormalizeVPN(base::DictionaryValue* vpn);
void NormalizeWiFi(base::DictionaryValue* wifi);
void NormalizeStaticIPConfigForNetwork(base::DictionaryValue* network);
const bool remove_recommended_fields_;
......
......@@ -31,6 +31,58 @@ TEST(ONCNormalizerTest, RemoveStaticIPConfig) {
EXPECT_TRUE(test_utils::Equals(expected_normalized, actual_normalized.get()));
}
// Validate that irrelevant fields of the StaticIPConfig dictionary will be
// removed.
TEST(ONCNormalizerTest, RemoveStaticIPConfigFields) {
Normalizer normalizer(true);
std::unique_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-fields", &original);
data->GetDictionary("irrelevant-staticipconfig-fields-normalized",
&expected_normalized);
std::unique_ptr<base::DictionaryValue> actual_normalized =
normalizer.NormalizeObject(&kNetworkConfigurationSignature, *original);
EXPECT_TRUE(test_utils::Equals(expected_normalized, actual_normalized.get()));
}
// Validate that StatticIPConfig.NameServers is removed when
// NameServersConfigType is 'DHCP'.
TEST(ONCNormalizerTest, RemoveNameServers) {
Normalizer normalizer(true);
std::unique_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-nameservers", &original);
data->GetDictionary("irrelevant-nameservers-normalized",
&expected_normalized);
std::unique_ptr<base::DictionaryValue> actual_normalized =
normalizer.NormalizeObject(&kNetworkConfigurationSignature, *original);
EXPECT_TRUE(test_utils::Equals(expected_normalized, actual_normalized.get()));
}
// Validate that IPConfig related fields are removed when IPAddressConfigType
// is 'Static', but some required fields are missing.
TEST(ONCNormalizerTest, RemoveIPFieldsForIncompleteConfig) {
Normalizer normalizer(true);
std::unique_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("missing-ip-fields", &original);
data->GetDictionary("missing-ip-fields-normalized", &expected_normalized);
std::unique_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);
......
......@@ -189,6 +189,7 @@ const OncFieldSignature tether_with_state_fields[] = {
{NULL}};
const OncFieldSignature ipconfig_fields[] = {
{::onc::kRecommended, &kRecommendedSignature},
{::onc::ipconfig::kGateway, &kStringSignature},
{::onc::ipconfig::kIPAddress, &kStringSignature},
{::onc::ipconfig::kNameServers, &kStringListSignature},
......
......@@ -44,6 +44,24 @@ std::string GetStringFromDict(const base::Value& dict, const char* key) {
return value ? value->GetString() : std::string();
}
bool FieldIsRecommended(const base::DictionaryValue& object,
const std::string& field_name) {
const base::Value* recommended =
object.FindKeyOfType(::onc::kRecommended, base::Value::Type::LIST);
return recommended &&
base::ContainsValue(recommended->GetList(), base::Value(field_name));
}
bool FieldIsSetToValueOrRecommended(const base::DictionaryValue& object,
const std::string& field_name,
const base::Value& expected_value) {
const base::Value* actual_value = object.FindKey(field_name);
if (actual_value && expected_value == *actual_value)
return true;
return FieldIsRecommended(object, field_name);
}
} // namespace
Validator::Validator(bool error_on_unknown_field,
......@@ -116,8 +134,7 @@ std::unique_ptr<base::DictionaryValue> Validator::MapObject(
} else if (&signature == &kEthernetSignature) {
valid = ValidateEthernet(repaired.get());
} else if (&signature == &kIPConfigSignature ||
&signature == &kSavedIPConfigSignature ||
&signature == &kStaticIPConfigSignature) {
&signature == &kSavedIPConfigSignature) {
valid = ValidateIPConfig(repaired.get());
} else if (&signature == &kWiFiSignature) {
valid = ValidateWiFi(repaired.get());
......@@ -148,6 +165,10 @@ std::unique_ptr<base::DictionaryValue> Validator::MapObject(
} else if (&signature == &kTetherWithStateSignature) {
valid = ValidateTether(repaired.get());
}
// StaticIPConfig is not validated here, because its correctness depends
// on NetworkConfiguration's 'IPAddressConfigType', 'NameServersConfigType'
// and 'Recommended' fields. It's validated in
// ValidateNetworkConfiguration() instead.
}
if (valid)
......@@ -426,6 +447,19 @@ bool Validator::FieldExistsAndIsEmpty(const base::DictionaryValue& object,
return true;
}
bool Validator::FieldShouldExistOrBeRecommended(
const base::DictionaryValue& object,
const std::string& field_name) {
if (object.HasKey(field_name) || FieldIsRecommended(object, field_name))
return true;
std::ostringstream msg;
msg << "Field " << field_name << " is not found, but expected either to be "
<< "set or to be recommended.";
AddValidationIssue(error_on_missing_field_, msg.str());
return !error_on_missing_field_;
}
bool Validator::OnlyOneFieldSet(const base::DictionaryValue& object,
const std::string& field_name1,
const std::string& field_name2) {
......@@ -617,16 +651,8 @@ bool Validator::ValidateNetworkConfiguration(base::DictionaryValue* result) {
all_required_exist &=
RequireField(*result, kName) && RequireField(*result, kType);
std::string ip_address_config_type =
GetStringFromDict(*result, kIPAddressConfigType);
std::string name_servers_config_type =
GetStringFromDict(*result, kNameServersConfigType);
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);
}
if (!NetworkHasCorrectStaticIPConfig(result))
return false;
std::string type = GetStringFromDict(*result, kType);
......@@ -683,7 +709,8 @@ bool Validator::ValidateEthernet(base::DictionaryValue* result) {
return !error_on_missing_field_ || all_required_exist;
}
bool Validator::ValidateIPConfig(base::DictionaryValue* result) {
bool Validator::ValidateIPConfig(base::DictionaryValue* result,
bool require_fields) {
using namespace ::onc::ipconfig;
const char* const kValidTypes[] = {kIPv4, kIPv6};
......@@ -701,15 +728,49 @@ bool Validator::ValidateIPConfig(base::DictionaryValue* result) {
return false;
}
bool all_required_exist = RequireField(*result, kIPAddress) &&
RequireField(*result, ::onc::ipconfig::kType);
if (result->HasKey(kIPAddress))
bool all_required_exist = true;
if (require_fields) {
all_required_exist &= RequireField(*result, kIPAddress);
all_required_exist &= RequireField(*result, kRoutingPrefix);
all_required_exist &= RequireField(*result, kGateway);
} else {
all_required_exist &= FieldShouldExistOrBeRecommended(*result, kIPAddress);
all_required_exist &=
FieldShouldExistOrBeRecommended(*result, kRoutingPrefix);
all_required_exist &= FieldShouldExistOrBeRecommended(*result, kGateway);
}
return !error_on_missing_field_ || all_required_exist;
}
bool Validator::NetworkHasCorrectStaticIPConfig(
base::DictionaryValue* network) {
using namespace ::onc::network_config;
using namespace ::onc::ipconfig;
bool must_have_ip_config = FieldIsSetToValueOrRecommended(
*network, kIPAddressConfigType, base::Value(kIPConfigTypeStatic));
bool must_have_nameservers = FieldIsSetToValueOrRecommended(
*network, kNameServersConfigType, base::Value(kIPConfigTypeStatic));
if (!must_have_ip_config && !must_have_nameservers)
return true;
if (!RequireField(*network, kStaticIPConfig))
return false;
base::DictionaryValue* static_ip_config = nullptr;
network->GetDictionary(kStaticIPConfig, &static_ip_config);
bool valid = true;
// StaticIPConfig should have all fields required by the corresponding
// IPAddressConfigType and NameServersConfigType values.
if (must_have_ip_config)
valid &= ValidateIPConfig(static_ip_config, false /* require_fields */);
if (must_have_nameservers)
valid &= FieldShouldExistOrBeRecommended(*static_ip_config, kNameServers);
return valid;
}
bool Validator::ValidateWiFi(base::DictionaryValue* result) {
using namespace ::onc::wifi;
......
......@@ -175,7 +175,9 @@ class CHROMEOS_EXPORT Validator : public Mapper {
bool ValidateToplevelConfiguration(base::DictionaryValue* result);
bool ValidateNetworkConfiguration(base::DictionaryValue* result);
bool ValidateEthernet(base::DictionaryValue* result);
bool ValidateIPConfig(base::DictionaryValue* result);
bool ValidateIPConfig(base::DictionaryValue* result,
bool require_fields = true);
bool ValidateNameServersConfig(base::DictionaryValue* result);
bool ValidateWiFi(base::DictionaryValue* result);
bool ValidateVPN(base::DictionaryValue* result);
bool ValidateIPsec(base::DictionaryValue* result);
......@@ -210,6 +212,17 @@ class CHROMEOS_EXPORT Validator : public Mapper {
bool FieldExistsAndIsEmpty(const base::DictionaryValue& object,
const std::string& field_name);
// Validates 'StaticIPConfig' field of the given network configuration. This
// method takes 'NetworkConfiguration' dict instead of 'StaticIPConfig' dict
// because it needs other 'NetworkConfiguration' fields (e.g.
// 'IPAddressConfigType' and 'NameServersConfigType') to check correctness of
// the 'StaticIPConfig' field.
bool NetworkHasCorrectStaticIPConfig(base::DictionaryValue* network);
// Validates that the given field either exists or is recommended.
bool FieldShouldExistOrBeRecommended(const base::DictionaryValue& object,
const std::string& field_name);
bool OnlyOneFieldSet(const base::DictionaryValue& object,
const std::string& field_name1,
const std::string& field_name2);
......
......@@ -92,6 +92,7 @@
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
"IPAddressConfigType": "Static",
"StaticIPConfig": {
"Type": "IPv4",
"IPAddress": "127.0.0.1",
......@@ -235,6 +236,7 @@
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
"IPAddressConfigType": "Static",
"StaticIPConfig": {
"Type": "IPv4",
"IPAddress": "127.0.0.1",
......
......@@ -19,6 +19,74 @@
"Authentication": "None"
}
},
"irrelevant-staticipconfig-fields": {
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
"Ethernet": {
"Authentication": "None"
},
"NameServersConfigType": "Static",
"StaticIPConfig": {
"Gateway":"1.1.1.1",
"IPAddress": "127.0.0.1",
"NameServers": ["8.8.8.8"]
}
},
"irrelevant-staticipconfig-fields-normalized": {
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
"Ethernet": {
"Authentication": "None"
},
"NameServersConfigType": "Static",
"StaticIPConfig": {
"NameServers": ["8.8.8.8"]
}
},
"irrelevant-nameservers": {
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
"Ethernet": {
"Authentication": "None"
},
"NameServersConfigType": "DHCP",
"StaticIPConfig": {
"NameServers": ["8.8.8.8"]
}
},
"irrelevant-nameservers-normalized": {
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
"Ethernet": {
"Authentication": "None"
},
"NameServersConfigType": "DHCP"
},
"missing-ip-fields": {
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
"Ethernet": {
"Authentication": "None"
},
"IPAddressConfigType": "Static",
"StaticIPConfig": {
"IPAddress": "1.1.1.1",
"Gateway": "1.1.1.1"
}
},
"missing-ip-fields-normalized": {
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
"Ethernet": {
"Authentication": "None"
}
},
"ethernet-and-vpn": {
"Recommended": [],
"GUID": "guid",
......
......@@ -245,13 +245,10 @@ Field **NetworkConfigurations** is an array of
* (required if **IPAddressConfigType** or **NameServersConfigType** is set
to *Static*) - [IPConfig](#IPConfig-type)
* Each property set in this IPConfig object overrides the respective
parameter received over DHCP.
If **IPAddressConfigType** is set to
*Static*, **IPAddress**
and **Gateway** are required.
If **NameServersConfigType** is set to
*Static*, **NameServers**
is required.
parameter received over DHCP. If **IPAddressConfigType** is set to
*Static*, **IPAddress**, **Gateway** and **RoutingPrefix** are required.
If **NameServersConfigType** is set to *Static*, **NameServers** is
required.
* **SavedIPConfig**
* (optional for connected networks, read-only) - [IPConfig](#IPConfig-type)
......@@ -394,7 +391,7 @@ static IP configuration (see **StaticIPConfig**).
### IPConfig type
* **Type**
* (required) - **string**
* (optional, defaults to *IPv4*) - **string**
* Allowed values are:
* *IPv4*
* *IPv6*
......@@ -403,8 +400,8 @@ static IP configuration (see **StaticIPConfig**).
* **IPAddress**
* (optional) - **string**
* Describes the IPv4 or IPv6 address of a connection, depending on the value
of **Type** field. It should not contain the
routing prefix (i.e. should not end in something like /64).
of **Type** field. It should not contain the routing prefix (i.e. should
not end in something like /64).
* **RoutingPrefix**
* (required if **IPAddress** is set. Otherwise ignored.) - **integer**
......@@ -420,9 +417,8 @@ static IP configuration (see **StaticIPConfig**).
* **NameServers**
* (optional) - **array of string**
* Array of addresses to use for name servers. Address format must match that
specified in the **Type** field. If not specified,
DHCP values will be used.
* Array of addresses to use for name servers. If not specified, DHCP values
will be used.
* **SearchDomains**
* (optional) - **array of string**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment