Fix ONC password filtering in about:policy.

chrome/browser/policy/configuration_policy_handler_chromeos.cc

Index: chrome/browser/policy/configuration_policy_handler_chromeos.cc
diff --git a/chrome/browser/policy/configuration_policy_handler_chromeos.cc b/chrome/browser/policy/configuration_policy_handler_chromeos.cc
index d9edc6f439443d3e652c1ff08454baa6cde7692a..3f9f95e589474e97161eabdb724b2a95a26c7404 100644
--- a/chrome/browser/policy/configuration_policy_handler_chromeos.cc
+++ b/chrome/browser/policy/configuration_policy_handler_chromeos.cc
@@ -4,12 +4,15 @@
 
 #include "chrome/browser/policy/configuration_policy_handler_chromeos.h"

[pneubeck (no reviews), 2012/08/09 07:59:20]

Nit:
btw. the header is missing at least a forward declaration of DictionaryValue.
base/values.h should be included explicitly in .cc.
'base' namespace could be added to references to types/functions of values.h

 
+#include <algorithm>
 #include <string>
+#include <vector>
 
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/string_util.h"
+#include "chrome/browser/chromeos/cros/onc_constants.h"
 #include "chrome/browser/chromeos/cros/onc_network_parser.h"
 #include "chrome/browser/policy/policy_error_map.h"
 #include "chrome/browser/policy/policy_map.h"
@@ -19,6 +22,8 @@
 #include "grit/generated_resources.h"
 #include "policy/policy_constants.h"
 
+namespace onc = chromeos::onc;
+
 namespace policy {
 
 NetworkConfigurationPolicyHandler::NetworkConfigurationPolicyHandler(
@@ -111,21 +116,29 @@ Value* NetworkConfigurationPolicyHandler::SanitizeNetworkConfig(
 void NetworkConfigurationPolicyHandler::StripSensitiveValues(
     DictionaryValue* network_dict) {
   // List of settings we filter from the network dictionary.
-  static const char* kFilteredSettings[] = {
-    "WiFi.Passphrase",
-    "IPsec.EAP.Password",
-    "IPsec.EAP.Password",
-    "IPsec.XAUTH.Password",
-    "L2TP.Password",
+  std::vector<std::string> filtered_settings;

[pneubeck (no reviews), 2012/08/09 07:59:20]

Unused? If so, also remove the comment above.

[Mattias Nissler (ping if slow), 2012/08/09 09:09:14]

Done.

+  static const int kMaxComponents = 4;
+  static const char* kFilteredSettings[][kMaxComponents] = {

[pneubeck (no reviews), 2012/08/09 07:59:20]

Add a comment, that these are the components of a dictionary path which are to
be garbled. Each entry must be NULL-terminated.

Overall, the terms "filter" and "strip" are not used correctly, IMO. Instead the
sensitive data is "garbled"/"hidden"/"starred"...

[Mattias Nissler (ping if slow), 2012/08/09 09:09:14]

Done.

+    { onc::kEthernet, onc::ethernet::kEAP, onc::eap::kPassword },
+    { onc::kVPN, onc::vpn::kIPsec, onc::vpn::kPSK },
+    { onc::kVPN, onc::vpn::kL2TP, onc::vpn::kPassword },
+    { onc::kVPN, onc::vpn::kOpenVPN, onc::vpn::kPassword },
+    { onc::kWiFi, onc::wifi::kEAP, onc::eap::kPassword },
+    { onc::kWiFi, onc::wifi::kPassphrase },

[Joao da Silva, 2012/08/09 07:29:33]

Suggestion: align the 2nd and 3rd column entries?

[Mattias Nissler (ping if slow), 2012/08/09 09:09:14]

That would suggest the entries correspond to each other when in fact they're
not.

   };
+
   // Placeholder to insert in place of the filtered setting.
   static const char kPlaceholder[] = "********";
 
   for (size_t i = 0; i < arraysize(kFilteredSettings); ++i) {
-    if (network_dict->Remove(kFilteredSettings[i], NULL)) {
-      network_dict->Set(kFilteredSettings[i],
-                        Value::CreateStringValue(kPlaceholder));
-    }
+    std::vector<std::string> parts(
+        kFilteredSettings[i],
+        std::find(kFilteredSettings[i],
+                  kFilteredSettings[i] + kMaxComponents - 1,
+                  static_cast<const char*>(NULL)));

[Joao da Silva, 2012/08/09 07:29:33]

I'm not sure if the memory bots won't complain about reading uninitialized
values; maybe add explicit NULLs?

[Mattias Nissler (ping if slow), 2012/08/09 09:09:14]

The C++ standard says that missing elements in array initializers are
initialized to zeros.

+    std::string key(JoinString(parts, '.'));
+    if (network_dict->Remove(key, NULL))
+      network_dict->Set(key, Value::CreateStringValue(kPlaceholder));

[pneubeck (no reviews), 2012/08/09 07:59:20]

Maybe create the StringValue only once.

[Mattias Nissler (ping if slow), 2012/08/09 09:09:14]

No, we need fresh copies for each Set()

   }
 }