ONC: add support for non-utf-8 SSIDs

ONC currently uses strings to represent SSIDs. Since JSON-Schema strings are restricted to Unicode and ONC is encoding these as UTF-8, this means that ONC cannot be used to represent SSIDs that include non-UTF-8 strings.

This change adds a separate HexSSID field to the WiFi section of the ONC that expects an SSID byte sequence encoded in hex.

At least one of the fields HexSSID or SSID must be present. If both HexSSID and SSID are set, the values must be consistent.

BUG=432546

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

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

17 files changed, 275 insertions, 6 deletions

diff --git a/chromeos/network/managed_network_configuration_handler_impl.cc b/chromeos/network/managed_network_configuration_handler_impl.cc
index eb16c55..11d33f4 100644
--- a/chromeos/network/managed_network_configuration_handler_impl.cc
+++ b/chromeos/network/managed_network_configuration_handler_impl.cc
@@ -32,6 +32,7 @@
 #include "chromeos/network/onc/onc_merger.h"
 #include "chromeos/network/onc/onc_signature.h"
 #include "chromeos/network/onc/onc_translator.h"
+#include "chromeos/network/onc/onc_utils.h"
 #include "chromeos/network/onc/onc_validator.h"
 #include "chromeos/network/policy_util.h"
 #include "chromeos/network/shill_property_util.h"
@@ -289,6 +290,13 @@ void ManagedNetworkConfigurationHandlerImpl::SetProperties(
           *user_settings_copy,
           &validation_result);
 
+  // Fill in HexSSID field from contents of SSID field if not set already.
+  if (user_settings_copy) {
+    onc::FillInHexSSIDFieldsInOncObject(
+        onc::kNetworkConfigurationSignature, validated_user_settings.get());
+  }
+
+
   if (validation_result == onc::Validator::INVALID) {
     InvokeErrorCallback(service_path, error_callback, kInvalidUserSettings);
     return;
diff --git a/chromeos/network/onc/onc_normalizer.cc b/chromeos/network/onc/onc_normalizer.cc
index f93882f..abe93e0 100644
--- a/chromeos/network/onc/onc_normalizer.cc
+++ b/chromeos/network/onc/onc_normalizer.cc
@@ -9,6 +9,7 @@
 #include "base/logging.h"
 #include "base/values.h"
 #include "chromeos/network/onc/onc_signature.h"
+#include "chromeos/network/onc/onc_utils.h"
 #include "components/onc/onc_constants.h"
 
 namespace chromeos {
@@ -232,6 +233,7 @@ void Normalizer::NormalizeWiFi(base::DictionaryValue* wifi) {
   RemoveEntryUnless(wifi, kEAP, security == kWEP_8021X || security == kWPA_EAP);
   RemoveEntryUnless(wifi, kPassphrase,
                     security == kWEP_PSK || security == kWPA_PSK);
+  FillInHexSSIDField(wifi);
 }
 
 }  // namespace onc
diff --git a/chromeos/network/onc/onc_normalizer.h b/chromeos/network/onc/onc_normalizer.h
index 7268879..9c97d32 100644
--- a/chromeos/network/onc/onc_normalizer.h
+++ b/chromeos/network/onc/onc_normalizer.h
@@ -24,7 +24,10 @@ class CHROMEOS_EXPORT Normalizer : public Mapper {
   // type is "Ethernet". If |remove_recommended_fields| is true, kRecommended
   // arrays are removed (the array itself and not the field names listed
   // there). |object_signature| must point to one of the signatures in
-  // |onc_signature.h|.
+  // |onc_signature.h|. For configurations of type "WiFi", if the "SSID" field
+  // is set, but the field "HexSSID" is not, the contents of the "SSID" field is
+  // converted to UTF-8 encoding, a hex representation of the byte sequence is
+  // created and stored in the field "HexSSID".
   scoped_ptr<base::DictionaryValue> NormalizeObject(
       const OncValueSignature* object_signature,
       const base::DictionaryValue& onc_object);
diff --git a/chromeos/network/onc/onc_normalizer_unittest.cc b/chromeos/network/onc/onc_normalizer_unittest.cc
index 022f33f..bf80d42 100644
--- a/chromeos/network/onc/onc_normalizer_unittest.cc
+++ b/chromeos/network/onc/onc_normalizer_unittest.cc
@@ -14,7 +14,7 @@ namespace chromeos {
 namespace onc {
 
 // This test case is about validating valid ONC objects.
-TEST(ONCNormalizerTest, NormalizeNetworkConfiguration) {
+TEST(ONCNormalizerTest, NormalizeNetworkConfigurationEthernetAndVPN) {
   Normalizer normalizer(true);
   scoped_ptr<const base::DictionaryValue> data(
       test_utils::ReadTestDictionary("settings_with_normalization.json"));
@@ -29,5 +29,21 @@ TEST(ONCNormalizerTest, NormalizeNetworkConfiguration) {
   EXPECT_TRUE(test_utils::Equals(expected_normalized, actual_normalized.get()));
 }
 
+// This test case is about validating valid ONC objects.
+TEST(ONCNormalizerTest, NormalizeNetworkConfigurationWifi) {
+  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("wifi", &original);
+  data->GetDictionary("wifi-normalized", &expected_normalized);
+
+  scoped_ptr<base::DictionaryValue> actual_normalized =
+      normalizer.NormalizeObject(&kNetworkConfigurationSignature, *original);
+  EXPECT_TRUE(test_utils::Equals(expected_normalized, actual_normalized.get()));
+}
+
 }  // namespace onc
 }  // namespace chromeos
diff --git a/chromeos/network/onc/onc_signature.cc b/chromeos/network/onc/onc_signature.cc
index b6b7a2e..2bd48ea 100644
--- a/chromeos/network/onc/onc_signature.cc
+++ b/chromeos/network/onc/onc_signature.cc
@@ -206,6 +206,7 @@ const OncFieldSignature wifi_fields[] = {
     { ::onc::wifi::kAllowGatewayARPPolling, &kBoolSignature},
     { ::onc::wifi::kAutoConnect, &kBoolSignature},
     { ::onc::wifi::kEAP, &kEAPSignature},
+    { ::onc::wifi::kHexSSID, &kStringSignature},
     { ::onc::wifi::kHiddenSSID, &kBoolSignature},
     { ::onc::wifi::kPassphrase, &kStringSignature},
     { ::onc::wifi::kSSID, &kStringSignature},
diff --git a/chromeos/network/onc/onc_utils.cc b/chromeos/network/onc/onc_utils.cc
index fdff42d..7b544cf 100644
--- a/chromeos/network/onc/onc_utils.cc
+++ b/chromeos/network/onc/onc_utils.cc
@@ -8,6 +8,7 @@
 #include "base/json/json_reader.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "chromeos/network/network_event_log.h"
@@ -240,6 +241,39 @@ void ExpandStringsInNetworks(const StringSubstitution& substitution,
   }
 }
 
+void FillInHexSSIDFieldsInOncObject(const OncValueSignature& signature,
+                                    base::DictionaryValue* onc_object) {
+  if (&signature == &kWiFiSignature)
+    FillInHexSSIDField(onc_object);
+
+  // Recurse into nested objects.
+  for (base::DictionaryValue::Iterator it(*onc_object); !it.IsAtEnd();
+       it.Advance()) {
+    base::DictionaryValue* inner_object = nullptr;
+    if (!onc_object->GetDictionaryWithoutPathExpansion(it.key(), &inner_object))
+      continue;
+
+    const OncFieldSignature* field_signature =
+        GetFieldSignature(signature, it.key());
+    if (!field_signature)
+      continue;
+
+    FillInHexSSIDFieldsInOncObject(*field_signature->value_signature,
+                                   inner_object);
+  }
+}
+
+void FillInHexSSIDField(base::DictionaryValue* wifi_fields) {
+  if (!wifi_fields->HasKey(::onc::wifi::kHexSSID)) {
+    std::string ssid_string;
+    wifi_fields->GetStringWithoutPathExpansion(::onc::wifi::kSSID,
+                                               &ssid_string);
+    wifi_fields->SetStringWithoutPathExpansion(
+        ::onc::wifi::kHexSSID,
+        base::HexEncode(ssid_string.c_str(), ssid_string.size()));
+  }
+}
+
 namespace {
 
 class OncMaskValues : public Mapper {
@@ -398,6 +432,9 @@ bool ParseAndValidateOncForImport(const std::string& onc_blob,
       *toplevel_onc,
       &validation_result);
 
+  FillInHexSSIDFieldsInOncObject(kToplevelConfigurationSignature,
+                                 toplevel_onc.get());
+
   if (from_policy) {
     UMA_HISTOGRAM_BOOLEAN("Enterprise.ONC.PolicyValidation",
                           validation_result == Validator::VALID);
diff --git a/chromeos/network/onc/onc_utils.h b/chromeos/network/onc/onc_utils.h
index c31cd2e..0e2122e 100644
--- a/chromeos/network/onc/onc_utils.h
+++ b/chromeos/network/onc/onc_utils.h
@@ -83,6 +83,17 @@ CHROMEOS_EXPORT void ExpandStringsInNetworks(
     const StringSubstitution& substitution,
     base::ListValue* network_configs);
 
+// Fills in all missing HexSSID fields that are mentioned in the ONC
+// specification. The object of |onc_object| is modified in place.
+CHROMEOS_EXPORT void FillInHexSSIDFieldsInOncObject(
+    const OncValueSignature& signature,
+    base::DictionaryValue* onc_object);
+
+// If the SSID field is set, but HexSSID is not, converts the contents of the
+// SSID field to UTF-8 encoding, creates the hex representation and assigns the
+// result to HexSSID.
+CHROMEOS_EXPORT void FillInHexSSIDField(base::DictionaryValue* wifi_fields);
+
 // Creates a copy of |onc_object| with all values of sensitive fields replaced
 // by |mask|. To find sensitive fields, signature and field name are checked
 // with the function FieldIsCredential().
diff --git a/chromeos/network/onc/onc_validator.cc b/chromeos/network/onc/onc_validator.cc
index d84d255..fd55b29 100644
--- a/chromeos/network/onc/onc_validator.cc
+++ b/chromeos/network/onc/onc_validator.cc
@@ -20,6 +20,9 @@ namespace onc {
 
 namespace {
 
+// According to the IEEE 802.11 standard the SSID is a series of 0 to 32 octets.
+const int kMaximumSSIDLengthInBytes = 32;
+
 template <typename T, size_t N>
 std::vector<T> toVector(T const (&array)[N]) {
   return std::vector<T>(array, array + N);
@@ -396,6 +399,54 @@ bool Validator::FieldExistsAndIsEmpty(const base::DictionaryValue& object,
   return true;
 }
 
+bool Validator::IsSSIDOrHexSSIDValid(const base::DictionaryValue& object) {
+  // Check SSID validity.
+  std::string ssid_string;
+  if (object.GetStringWithoutPathExpansion(::onc::wifi::kSSID, &ssid_string)) {
+    if (ssid_string.size() <= 0 ||
+        ssid_string.size() > kMaximumSSIDLengthInBytes) {
+      LOG(ERROR) << MessageHeader() << ::onc::wifi::kSSID
+                 << " has an invalid length.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+  }
+
+  // Check HexSSID validity.
+  std::string hex_ssid_string;
+  if (object.GetStringWithoutPathExpansion(::onc::wifi::kHexSSID,
+                                           &hex_ssid_string)) {
+    std::vector<uint8> bytes;
+    if (!base::HexStringToBytes(hex_ssid_string, &bytes)) {
+      LOG(ERROR) << MessageHeader() << "Field " << ::onc::wifi::kHexSSID
+                 << " is not a valid hex representation: \"" << hex_ssid_string
+                 << "\"";
+      error_or_warning_found_ = true;
+      return false;
+    }
+    if (bytes.size() <= 0 || bytes.size() > kMaximumSSIDLengthInBytes) {
+      LOG(ERROR) << MessageHeader() << ::onc::wifi::kHexSSID
+                 << " has an invalid length.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+  }
+  // If both SSID and HexSSID are set, ensure that they are consistent.
+  if (ssid_string.length() > 0 && hex_ssid_string.length() > 0) {
+    std::string hexified =
+        base::HexEncode(ssid_string.c_str(), ssid_string.size());
+    if (hexified != hex_ssid_string) {
+      LOG(ERROR) << MessageHeader() << "Fields " << ::onc::wifi::kSSID
+                 << " and " << ::onc::wifi::kHexSSID
+                 << " contain inconsistent values.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+  }
+
+  return true;
+}
+
 bool Validator::RequireField(const base::DictionaryValue& dict,
                              const std::string& field_name) {
   if (dict.HasKey(field_name))
@@ -572,8 +623,17 @@ bool Validator::ValidateWiFi(base::DictionaryValue* result) {
   if (FieldExistsAndHasNoValidValue(*result, kSecurity, valid_securities))
     return false;
 
-  bool all_required_exist =
-      RequireField(*result, kSecurity) && RequireField(*result, kSSID);
+  // Validate SSID and HexSSID fields, if present.
+  if (!IsSSIDOrHexSSIDValid(*result))
+    return false;
+
+  bool all_required_exist = RequireField(*result, kSecurity);
+
+  // One of {kSSID, kHexSSID} must be present.
+  if (!result->HasKey(kSSID))
+    all_required_exist &= RequireField(*result, kHexSSID);
+  if (!result->HasKey(kHexSSID))
+    all_required_exist &= RequireField(*result, kSSID);
 
   std::string security;
   result->GetStringWithoutPathExpansion(kSecurity, &security);
diff --git a/chromeos/network/onc/onc_validator.h b/chromeos/network/onc/onc_validator.h
index 0fb60eb..857f31e 100644
--- a/chromeos/network/onc/onc_validator.h
+++ b/chromeos/network/onc/onc_validator.h
@@ -182,6 +182,8 @@ class CHROMEOS_EXPORT Validator : public Mapper {
   bool FieldExistsAndIsEmpty(const base::DictionaryValue& object,
                              const std::string& field_name);
 
+  bool IsSSIDOrHexSSIDValid(const base::DictionaryValue& object);
+
   // Returns true if |key| is a key of |dict|. Otherwise, returns false and,
   // depending on |error_on_missing_field_|, logs a message and sets
   // |error_or_warning_found_|.
diff --git a/chromeos/network/onc/onc_validator_unittest.cc b/chromeos/network/onc/onc_validator_unittest.cc
index c109ddc..4daa6a6 100644
--- a/chromeos/network/onc/onc_validator_unittest.cc
+++ b/chromeos/network/onc/onc_validator_unittest.cc
@@ -152,6 +152,12 @@ INSTANTIATE_TEST_CASE_P(
         OncParams("managed_toplevel_l2tpipsec.onc",
                   &kToplevelConfigurationSignature,
                   true),
+        OncParams("toplevel_wifi_hexssid.onc",
+                  &kToplevelConfigurationSignature,
+                  false),
+        OncParams("toplevel_wifi_ssid_and_hexssid.onc",
+                  &kToplevelConfigurationSignature,
+                  false),
         OncParams("toplevel_wifi_wpa_psk.onc",
                   &kToplevelConfigurationSignature,
                   false),
@@ -449,6 +455,18 @@ INSTANTIATE_TEST_CASE_P(
                                  &kNetworkConfigurationSignature,
                                  false),
                        ExpectBothNotValid("", "")),
+        std::make_pair(OncParams("network-wifi-hexssid-invalid-length",
+                                 &kNetworkConfigurationSignature,
+                                 false),
+                       ExpectBothNotValid("", "")),
+        std::make_pair(OncParams("network-wifi-invalid-hexssid",
+                                 &kNetworkConfigurationSignature,
+                                 false),
+                       ExpectBothNotValid("", "")),
+        std::make_pair(OncParams("network-wifi-ssid-and-hexssid-inconsistent",
+                                 &kNetworkConfigurationSignature,
+                                 false),
+                       ExpectBothNotValid("", "")),
         std::make_pair(OncParams("managed-network-unknown-value",
                                  &kNetworkConfigurationSignature,
                                  true),
diff --git a/chromeos/test/data/network/invalid_settings_with_repairs.json b/chromeos/test/data/network/invalid_settings_with_repairs.json
index a26a8aa..5022c05 100644
--- a/chromeos/test/data/network/invalid_settings_with_repairs.json
+++ b/chromeos/test/data/network/invalid_settings_with_repairs.json
@@ -158,6 +158,37 @@
         },
         "ConnectionState": "NotConnected"
     },
+    "network-wifi-hexssid-invalid-length": {
+        "GUID": "guid",
+        "Type": "WiFi",
+        "Name": "name",
+        "WiFi": {
+            "Passphrase": "some passphrase",
+            "HexSSID": "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEFAB",
+            "Security": "WPA-PSK"
+        }
+    },
+    "network-wifi-invalid-hexssid": {
+        "GUID": "guid",
+        "Type": "WiFi",
+        "Name": "name",
+        "WiFi": {
+            "Passphrase": "some passphrase",
+            "HexSSID": "invalidhex",
+            "Security": "WPA-PSK"
+        }
+    },
+    "network-wifi-ssid-and-hexssid-inconsistent": {
+        "GUID": "guid",
+        "Type": "WiFi",
+        "Name": "name",
+        "WiFi": {
+            "Passphrase": "some passphrase",
+            "SSID": "ssid",
+            "HexSSID": "FFFFFF",
+            "Security": "WPA-PSK"
+        }
+    },
     "network-unknown-value": {
         "GUID": "guid",
         "Type": "LTE",
diff --git a/chromeos/test/data/network/settings_with_normalization.json b/chromeos/test/data/network/settings_with_normalization.json
index 08a78a4..7038e59 100644
--- a/chromeos/test/data/network/settings_with_normalization.json
+++ b/chromeos/test/data/network/settings_with_normalization.json
@@ -30,4 +30,24 @@
             "Authentication": "None"
         }
     },
+    "wifi": {
+      "GUID": "{77db0089-0bc8-4358-929c-123xcv}",
+      "Type": "WiFi",
+      "Name": "SomeWifi-XY",
+      "WiFi": {
+        "SSID": "SomeWifi-XY",
+        "Security": "None",
+        "Recommended": [ "AutoConnect" ]
+      }
+    },
+    "wifi-normalized": {
+      "GUID": "{77db0089-0bc8-4358-929c-123xcv}",
+      "Type": "WiFi",
+      "Name": "SomeWifi-XY",
+      "WiFi": {
+        "SSID": "SomeWifi-XY",
+        "HexSSID": "536F6D65576966692D5859",
+        "Security": "None"
+      }
+    }
 }
diff --git a/chromeos/test/data/network/toplevel_wifi_hexssid.onc b/chromeos/test/data/network/toplevel_wifi_hexssid.onc
new file mode 100644
index 0000000..471a0bb
--- /dev/null
+++ b/chromeos/test/data/network/toplevel_wifi_hexssid.onc
@@ -0,0 +1,16 @@
+// Test ONC file for importing an open network.
+{
+  "NetworkConfigurations": [
+    {
+      "GUID": "{485d6076-dd44-6b6d-69787465725f5040}",
+      "Type": "WiFi",
+      "Name": "My WiFi Network",
+      "WiFi": {
+        "HexSSID": "476F6F676C65",
+        "Security": "None"
+      }
+    }
+  ],
+  "Certificates": [],
+  "Type": "UnencryptedConfiguration"
+}
diff --git a/chromeos/test/data/network/toplevel_wifi_ssid_and_hexssid.onc b/chromeos/test/data/network/toplevel_wifi_ssid_and_hexssid.onc
new file mode 100644
index 0000000..99be5bc
--- /dev/null
+++ b/chromeos/test/data/network/toplevel_wifi_ssid_and_hexssid.onc
@@ -0,0 +1,17 @@
+// Test ONC file for importing an open network.
+{
+  "NetworkConfigurations": [
+    {
+      "GUID": "{485d6076-dd44-6b6d-69787465725f5040}",
+      "Type": "WiFi",
+      "Name": "My WiFi Network",
+      "WiFi": {
+        "SSID": "Google",
+        "HexSSID": "476F6F676C65",
+        "Security": "None"
+      }
+    }
+  ],
+  "Certificates": [],
+  "Type": "UnencryptedConfiguration"
+}
diff --git a/components/onc/docs/onc_spec.html b/components/onc/docs/onc_spec.html
index f4e70c8..a5e27eb 100644
--- a/components/onc/docs/onc_spec.html
+++ b/components/onc/docs/onc_spec.html
@@ -669,6 +669,21 @@
       EAP settings.
     </dd>
 
+    <dt class="field">HexSSID</dt>
+    <dd>
+      <span class="field_meta">
+        (optional if <span class="field">SSID</span> is set, if so defaults to
+        a hex representation of <span class="field">SSID</span>)
+        <span class="type">string</span>
+      </span>
+      Hex representation of the network's SSID. If the
+      <span class="field">SSID</span> field is set, but
+      <span class="field">HexSSID</span> is not, converts the contents of the
+      <span class="field">SSID</span> field to UTF-8 encoding,
+      creates the hex representation and assigns the result to
+      <span class="field">HexSSID</span>.
+    </dd>
+
     <dt class="field">HiddenSSID</dt>
     <dd>
       <span class="field_meta">
@@ -711,10 +726,12 @@
     <dt class="field">SSID</dt>
     <dd>
       <span class="field_meta">
-        (required)
+        (optional if <span class="field">HexSSID</span> is set)
         <span class="type">string</span>
       </span>
-      SSID of the network.
+      SSID of the network. This field can only used for unicode strings in the
+      UTF-8 encoding. For non-UTF-8 encodings the
+      <span class="field">HexSSID</span> field must be used.
     </dd>
 
     <dt class="field">SignalStrength</dt>
@@ -728,6 +745,14 @@
       be set to '0' or not present.
     </dd>
   </dl>
+  <span class="rule">
+    <span class="rule_id"></span>
+      At least one of the fields <span class="field">HexSSID</span> or
+      <span class="field">SSID</span> must be present. If both
+      <span class="field">HexSSID</span> and <span class="field">SSID</span>
+      are set, the values must be consistent.
+    </span>
+  </span>
 </section>
 
 <section>
diff --git a/components/onc/onc_constants.cc b/components/onc/onc_constants.cc
index e9d1905..51dfbc5 100644
--- a/components/onc/onc_constants.cc
+++ b/components/onc/onc_constants.cc
@@ -183,6 +183,7 @@ const char kBSSID[] = "BSSID";
 const char kEAP[] = "EAP";
 const char kFrequency[] = "Frequency";
 const char kFrequencyList[] = "FrequencyList";
+const char kHexSSID[] = "HexSSID";
 const char kHiddenSSID[] = "HiddenSSID";
 const char kPassphrase[] = "Passphrase";
 const char kSSID[] = "SSID";
diff --git a/components/onc/onc_constants.h b/components/onc/onc_constants.h
index 89b193b..9b9d1ad 100644
--- a/components/onc/onc_constants.h
+++ b/components/onc/onc_constants.h
@@ -201,6 +201,7 @@ ONC_EXPORT extern const char kBSSID[];
 ONC_EXPORT extern const char kEAP[];
 ONC_EXPORT extern const char kFrequency[];
 ONC_EXPORT extern const char kFrequencyList[];
+ONC_EXPORT extern const char kHexSSID[];
 ONC_EXPORT extern const char kHiddenSSID[];
 ONC_EXPORT extern const char kPassphrase[];
 ONC_EXPORT extern const char kSSID[];