ONC: add support for non-utf-8 SSIDs

chromeos/network/onc/onc_validator.cc

 // Copyright (c) 2012 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 "chromeos/network/onc/onc_validator.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "chromeos/network/onc/onc_signature.h"
 #include "components/onc/onc_constants.h"
 
 namespace chromeos {
 namespace onc {
 
 namespace {
 
+const int kMaximumSSIDLengthInBytes = 32;

[pneubeck (no reviews), 2014/11/26 10:15:34]

nit: add a comment where this comes from (802.11 standard)

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+
 template <typename T, size_t N>
 std::vector<T> toVector(T const (&array)[N]) {
   return std::vector<T>(array, array + N);
 }
 
 // Copied from policy/configuration_policy_handler.cc.
 // TODO(pneubeck): move to a common place like base/.
 std::string ValueTypeToString(base::Value::Type type) {
   const char* const strings[] = {"null",   "boolean", "integer",    "double",
                                  "string", "binary",  "dictionary", "list"};
@@ skipping 356 matching lines @@
   }
 
   error_or_warning_found_ = true;
   path_.push_back(field_name);
   LOG(ERROR) << MessageHeader() << "Found an empty string, but expected a "
              << "non-empty string.";
   path_.pop_back();
   return true;
 }
 
+bool Validator::IsSSIDOrHexSSIDValid(const base::DictionaryValue& object,
+                                     const std::string& ssid_field_name,

[pneubeck (no reviews), 2014/11/26 10:15:34]

as you always pass the same strings for the field names, you could inline kSSID
and kHexSSID

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+                                     const std::string& hex_ssid_field_name) {
+  // SSID validity

[pneubeck (no reviews), 2014/11/26 10:15:34]

nit: use whole sentences (ending with fullstop).

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+  std::string ssid_string;
+  const base::Value* value = NULL;
+  if (object.GetWithoutPathExpansion(ssid_field_name, &value)) {
+    if (!value->GetAsString(&ssid_string)) {

[pneubeck (no reviews), 2014/11/26 10:15:34]

type is verified already by the generic/default verification _before_ the custom
verification code runs.

you can change line 407 to 'GetStringWithoutPathExpansion'

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+      LOG(ERROR) << MessageHeader() << "Field " << ssid_field_name
+                 << " does not contain a string.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+    if (ssid_string.size() <= 0 ||
+        ssid_string.size() > kMaximumSSIDLengthInBytes) {
+      LOG(ERROR) << MessageHeader() << ssid_field_name
+                 << " has an invalid length.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+  }
+
+  // HexSSID validity
+  std::string hex_ssid_string;
+  if (object.GetWithoutPathExpansion(hex_ssid_field_name, &value)) {

[pneubeck (no reviews), 2014/11/26 10:15:34]

use GetStringWithoutPathExpansion

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+    if (!value->GetAsString(&hex_ssid_string)) {

[pneubeck (no reviews), 2014/11/26 10:15:34]

again not required

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+      LOG(ERROR) << MessageHeader() << "Field " << hex_ssid_field_name
+                 << " does not contain a string.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+    std::vector<uint8> bytes;
+    if (!base::HexStringToBytes(hex_ssid_string, &bytes)) {
+      LOG(ERROR) << MessageHeader() << "Field " << hex_ssid_field_name
+                 << " 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() << hex_ssid_field_name
+                 << " has an invalid length.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+  }
+
+  if (ssid_string.length() > 0 && hex_ssid_string.length() > 0) {
+    // both set

[pneubeck (no reviews), 2014/11/26 10:15:34]

nit: make the comment a bit more verbose:

"If both SSID and HexSSID are set, verify that they are consistent."

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+    std::string hexified =
+        base::HexEncode(ssid_string.c_str(), ssid_string.size());
+    if (hexified != hex_ssid_string) {
+      LOG(ERROR) << MessageHeader() << "Fields " << ssid_field_name << " and "
+                 << hex_ssid_field_name << " 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))
     return true;
   std::string message = MessageHeader() + "The required field '" + field_name +
       "' is missing.";
   if (error_on_missing_field_) {
     error_or_warning_found_ = true;
     LOG(ERROR) << message;
   } else {
@@ skipping 156 matching lines @@
 
 bool Validator::ValidateWiFi(base::DictionaryValue* result) {
   using namespace ::onc::wifi;
 
   const char* const kValidSecurities[] = {kSecurityNone, kWEP_PSK, kWEP_8021X,
                                           kWPA_PSK, kWPA_EAP};
   const std::vector<const char*> valid_securities(toVector(kValidSecurities));
   if (FieldExistsAndHasNoValidValue(*result, kSecurity, valid_securities))
     return false;
 
-  bool all_required_exist =
-      RequireField(*result, kSecurity) && RequireField(*result, kSSID);
+  // validate SSID and HexSSID fields

[pneubeck (no reviews), 2014/11/26 10:15:34]

nit: start with capital case and end with fullstop

append a ', if present.' or 'if existent'

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+  if (!IsSSIDOrHexSSIDValid(*result, kSSID, kHexSSID))
+    return false;
+
+  bool all_required_exist = RequireField(*result, kSecurity);
+  if (!result->HasKey(kSSID))

[pneubeck (no reviews), 2014/11/26 10:15:34]

prepend a comment that says that "One of {kSSID, kHexSSID} must be present".

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+    all_required_exist &= RequireField(*result, kHexSSID);
+  if (!result->HasKey(kHexSSID))
+    all_required_exist &= RequireField(*result, kSSID);
 
   std::string security;
   result->GetStringWithoutPathExpansion(kSecurity, &security);
   if (security == kWEP_8021X || security == kWPA_EAP)
     all_required_exist &= RequireField(*result, kEAP);
   else if (security == kWEP_PSK || security == kWPA_PSK)
     all_required_exist &= RequireField(*result, kPassphrase);
 
   return !error_on_missing_field_ || all_required_exist;
 }
@@ skipping 257 matching lines @@
 }
 
 std::string Validator::MessageHeader() {
   std::string path = path_.empty() ? "toplevel" : JoinString(path_, ".");
   std::string message = "At " + path + ": ";
   return message;
 }
 
 }  // namespace onc
 }  // namespace chromeos