Skip to content

Commit

Permalink
Allow effectively empty ONC policies without throwing an error.
Browse files Browse the repository at this point in the history
The ONC validator threw an error if both the Certificates or NetworkConfigurations sections were missing in an ONC policy.
Now, we ignore that fact because we have to expect policies that only contain the new GlobalNetworkConfiguration section.

BUG=386182

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@279713 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
pneubeck@chromium.org committed Jun 25, 2014
1 parent 16738a8 commit 6bf29b2
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 25 deletions.
26 changes: 6 additions & 20 deletions chromeos/network/onc/onc_validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -434,30 +434,16 @@ bool Validator::ValidateToplevelConfiguration(base::DictionaryValue* result) {
if (FieldExistsAndHasNoValidValue(*result, kType, kValidTypes))
return false;

bool all_required_exist = true;

// Not part of the ONC spec yet:
// We don't require the type field and default to UnencryptedConfiguration.
std::string type = kUnencryptedConfiguration;
result->GetStringWithoutPathExpansion(kType, &type);
if (type == kUnencryptedConfiguration &&
!result->HasKey(kNetworkConfigurations) &&
!result->HasKey(kCertificates)) {
error_or_warning_found_ = true;
std::string message = MessageHeader() + "Neither the field '" +
kNetworkConfigurations + "' nor '" + kCertificates +
"is present, but at least one is required.";
if (error_on_missing_field_)
LOG(ERROR) << message;
else
LOG(WARNING) << message;
all_required_exist = false;
}
// Not part of the ONC spec:
// - We don't require the type field (we assume that it's an
// UnencryptedConfiguration then).
// - We don't require specific toplevel objects to be present (e.g. at least
// one of NetworkConfiguration or Certificates).

if (IsGlobalNetworkConfigInUserImport(*result))
return false;

return !error_on_missing_field_ || all_required_exist;
return true;
}

bool Validator::ValidateNetworkConfiguration(base::DictionaryValue* result) {
Expand Down
6 changes: 6 additions & 0 deletions chromeos/network/onc/onc_validator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,12 @@ INSTANTIATE_TEST_CASE_P(
OncParams("toplevel_openvpn_clientcert_with_cert_pems.onc",
&kToplevelConfigurationSignature,
false),
OncParams("toplevel_empty.onc",
&kToplevelConfigurationSignature,
false),
OncParams("toplevel_only_global_config.onc",
&kToplevelConfigurationSignature,
true),
OncParams("encrypted.onc", &kToplevelConfigurationSignature, true),
OncParams("managed_vpn.onc", &kNetworkConfigurationSignature, true),
OncParams("ethernet.onc", &kNetworkConfigurationSignature, true),
Expand Down
4 changes: 4 additions & 0 deletions chromeos/test/data/network/toplevel_empty.onc
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"Type": "UnencryptedConfiguration"
}

5 changes: 5 additions & 0 deletions chromeos/test/data/network/toplevel_only_global_config.onc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"GlobalNetworkConfiguration": {},
"Type": "UnencryptedConfiguration"
}

8 changes: 3 additions & 5 deletions components/onc/docs/onc_spec.html
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,9 @@ <h1>Unencrypted Configuration</h1>
</dd>
</dl>

<p class="rule">
<span class="rule_id"></span>
At least one array (either <span class="field">NetworkConfigurations</span>
and/or <span class="field">Certificates</span>) must be present.
</p>
Besides the field <span class="field">Type</span>, at least one other field
(<span class="field">NetworkConfigurations</span> or
<span class="field">Certificates</span>) should be present.

<section>
<h1>Network Configuration</h1>
Expand Down

0 comments on commit 6bf29b2

Please sign in to comment.