Change PolicyLoaderWin to load PReg files if possible.

chrome/browser/policy/policy_loader_win.cc

Index: chrome/browser/policy/policy_loader_win.cc
diff --git a/chrome/browser/policy/policy_loader_win.cc b/chrome/browser/policy/policy_loader_win.cc
index b527308a3b73dde7d3dda7a4d002808b615fa361..e478fc10a08da47b3139c9e3dac44129fbb3edc6 100644
--- a/chrome/browser/policy/policy_loader_win.cc
+++ b/chrome/browser/policy/policy_loader_win.cc
@@ -8,22 +8,28 @@
 
 #include <string.h>
 
+#include <rpc.h>  // For struct GUID
+#include <shlwapi.h>  // For PathIsUNC()
 #include <userenv.h>
 
-// userenv.dll is required for RegisterGPNotification().
+// userenv.dll is required for RegisterGPNotification() and GetAppliedGPOList().
 #pragma comment(lib, "userenv.lib")
+// shlwapi.dll is required for PathIsUNC().
+#pragma comment(lib, "shlwapi.lib")

[Joao da Silva, 2013/04/05 13:55:01]

nit: order

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

Done.

 
 #include "base/basictypes.h"
+#include "base/files/file_path.h"
 #include "base/json/json_reader.h"
 #include "base/logging.h"
-#include "base/memory/scoped_ptr.h"
 #include "base/string16.h"
+#include "base/string_util.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/sys_byteorder.h"
 #include "base/utf_string_conversions.h"
-#include "base/values.h"
 #include "base/win/registry.h"
 #include "chrome/browser/policy/policy_bundle.h"
 #include "chrome/browser/policy/policy_map.h"
+#include "chrome/browser/policy/preg_parser_win.h"
 #include "chrome/common/json_schema/json_schema_constants.h"
 #include "policy/policy_constants.h"
 
@@ -46,56 +52,69 @@ namespace registry_constants {
 
 namespace {
 
-// Map of registry hives to their corresponding policy scope, in decreasing
-// order of priority.
-const struct {
-  HKEY hive;
-  PolicyScope scope;
-} kHives[] = {
-  { HKEY_LOCAL_MACHINE,   POLICY_SCOPE_MACHINE  },
-  { HKEY_CURRENT_USER,    POLICY_SCOPE_USER     },
+const char kKeyMandatory[] = "policy";
+const char kKeyRecommended[] = "recommended";
+const char kKeySchema[] = "schema";
+const char kKeyThirdParty[] = "3rdparty";
+
+// The GUID of the registry settings group policy extension.
+GUID kRegistrySettingsCSEGUID = REGISTRY_EXTENSION_GUID;
+
+// The PReg file name.
+const base::FilePath::CharType kPRegFileName[] =
+    FILE_PATH_LITERAL("Registry.pol");
+
+// Deleter for using GPO lists with scoped_ptr.
+struct FreeGPOListDeleter {
+  inline void operator()(GROUP_POLICY_OBJECT* ptr) const {
+    FreeGPOList(ptr);
+  }
 };
 
-// Reads a REG_SZ string at |key| named |name| into |result|. Returns false if
-// the string could not be read.
-bool ReadRegistryString(RegKey* key,
-                        const string16& name,
-                        string16* result) {
-  DWORD value_size = 0;
-  DWORD key_type = 0;
-  scoped_array<uint8> buffer;
+// Returns the entry with key |name| in |dictionary| (can be NULL), or NULL.
+const base::DictionaryValue* GetEntry(const base::DictionaryValue* dictionary,
+                                      const std::string& name) {
+  if (!dictionary)
+    return NULL;
+  const base::DictionaryValue* entry = NULL;
+  dictionary->GetDictionaryWithoutPathExpansion(name, &entry);
+  return entry;
+}
 
-  if (key->ReadValue(name.c_str(), 0, &value_size, &key_type) != ERROR_SUCCESS)
-    return false;
-  if (key_type != REG_SZ)
-    return false;
+// Tries to extract the dictionary at |key| in |dict| and returns it.
+scoped_ptr<base::DictionaryValue> RemoveDict(base::DictionaryValue* dict,
+                                             const std::string& key) {
+  base::Value* entry = NULL;
+  base::DictionaryValue* result_dict = NULL;
+  if (dict && dict->RemoveWithoutPathExpansion(key, &entry) && entry)
+    entry->GetAsDictionary(&result_dict);

[Joao da Silva, 2013/04/05 13:55:01]

This leaks |entry| if it's not a dictionary.

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

Done.

 
-  // According to the Microsoft documentation, the string
-  // buffer may not be explicitly 0-terminated. Allocate a
-  // slightly larger buffer and pre-fill to zeros to guarantee
-  // the 0-termination.
-  buffer.reset(new uint8[value_size + 2]);
-  memset(buffer.get(), 0, value_size + 2);
-  key->ReadValue(name.c_str(), buffer.get(), &value_size, NULL);
-  result->assign(reinterpret_cast<const wchar_t*>(buffer.get()));
-  return true;
+  return make_scoped_ptr(result_dict);
 }
 
-// Reads a REG_DWORD integer at |key| named |name| into |result|. Returns false
-// if the value could no be read.
-bool ReadRegistryInteger(RegKey* key,
-                         const string16& name,
-                         uint32* result) {
-  DWORD dword;
-  if (key->ReadValueDW(name.c_str(), &dword) != ERROR_SUCCESS)
-    return false;
-  *result = dword;
-  return true;
+std::string GetSchemaTypeForValueType(base::Value::Type value_type) {
+  switch (value_type) {
+    case base::Value::TYPE_DICTIONARY:
+      return json_schema_constants::kObject;
+    case base::Value::TYPE_INTEGER:
+      return json_schema_constants::kInteger;
+    case base::Value::TYPE_LIST:
+      return json_schema_constants::kArray;
+    case base::Value::TYPE_BOOLEAN:
+      return json_schema_constants::kBoolean;
+    case base::Value::TYPE_STRING:
+      return json_schema_constants::kString;
+    default:
+      break;
+  }
+
+  NOTREACHED() << "Unsupported policy value type " << value_type;
+  return json_schema_constants::kNull;
 }
 
 // Returns the Value type described in |schema|, or |default_type| if not found.
-base::Value::Type GetType(const base::DictionaryValue* schema,
-                          base::Value::Type default_type) {
+base::Value::Type GetValueTypeForSchema(const base::DictionaryValue* schema,
+                                        base::Value::Type default_type) {
   // JSON-schema types to base::Value::Type mapping.
   static const struct {
     // JSON schema type.
@@ -115,7 +134,7 @@ base::Value::Type GetType(const base::DictionaryValue* schema,
   if (!schema)
     return default_type;
   std::string type;
-  if (!schema->GetString(schema::kType, &type))
+  if (!schema->GetStringWithoutPathExpansion(schema::kType, &type))
     return default_type;
   for (size_t i = 0; i < arraysize(kSchemaToValueTypeMap); ++i) {
     if (type == kSchemaToValueTypeMap[i].schema_type)
@@ -124,28 +143,11 @@ base::Value::Type GetType(const base::DictionaryValue* schema,
   return default_type;
 }
 
-// Returns the default type for registry entries of |reg_type|, when there is
-// no schema defined type for a policy.
-base::Value::Type GetDefaultFor(DWORD reg_type) {
-  return reg_type == REG_DWORD ? base::Value::TYPE_INTEGER :
-                                 base::Value::TYPE_STRING;
-}
-
-// Returns the entry with key |name| in |dictionary| (can be NULL), or NULL.
-const base::DictionaryValue* GetEntry(const base::DictionaryValue* dictionary,
-                                      const std::string& name) {
-  if (!dictionary)
-    return NULL;
-  const base::DictionaryValue* entry = NULL;
-  dictionary->GetDictionary(name, &entry);
-  return entry;
-}
-
 // Returns the schema for property |name| given the |schema| of an object.
 // Returns the "additionalProperties" schema if no specific schema for
 // |name| is present. Returns NULL if no schema is found.
 const base::DictionaryValue* GetSchemaFor(const base::DictionaryValue* schema,
-                                    const std::string& name) {
+                                          const std::string& name) {
   const base::DictionaryValue* properties =
       GetEntry(schema, schema::kProperties);
   const base::DictionaryValue* sub_schema = GetEntry(properties, name);
@@ -155,234 +157,285 @@ const base::DictionaryValue* GetSchemaFor(const base::DictionaryValue* schema,
   return GetEntry(schema, schema::kAdditionalProperties);
 }
 
-// Converts string |value| to another |type|, if possible.
-base::Value* ConvertStringValue(const string16& value, base::Value::Type type) {
-  switch (type) {
-    case base::Value::TYPE_NULL:
-      return base::Value::CreateNullValue();
-
-    case base::Value::TYPE_BOOLEAN: {
-      int int_value;
-      if (base::StringToInt(value, &int_value))
-        return base::Value::CreateBooleanValue(int_value != 0);
-      return NULL;
+// Reads the subtree of the Windows registry at |root| into the passed |dict|.
+void ReadRegistry(HKEY hive,
+                  const std::wstring& root,
+                  base::DictionaryValue* dict) {
+  // Open the key.
+  RegKey key(hive, root.c_str(), KEY_READ);
+  if (!key.Valid())
+    return;
+
+  // First, read all the values of the key.
+  for (RegistryValueIterator it(hive, root.c_str()); it.Valid(); ++it) {
+    const std::string name = UTF16ToUTF8(it.Name());
+    DWORD type;
+    if (key.ReadValue(it.Name(), NULL, 0, &type) == ERROR_SUCCESS) {
+      switch (type) {
+        case REG_SZ:
+        case REG_EXPAND_SZ: {
+          std::wstring wstring_value;
+          if (key.ReadValue(it.Name(), &wstring_value) == ERROR_SUCCESS) {

[Joao da Silva, 2013/04/05 13:55:01]

ReadValue() internally limits the value size to 1024 chars; the original
ReadRegistryString also handled non-NUL terminated strings.

I'd use ReadRegistryString() to read strings, am I missing something?

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

Good point. Instead of reviving ReadRegistryString, I saved a call to the
registry here given that we have a ReadValue call above already to find the
size.

+            dict->SetStringWithoutPathExpansion(name,
+                                                UTF16ToUTF8(wstring_value));
+            continue;
+          }
+          break;
+        }
+        case REG_DWORD_LITTLE_ENDIAN:
+        case REG_DWORD_BIG_ENDIAN: {
+          DWORD dword_value = 0;
+          if (key.ReadValueDW(it.Name(), &dword_value) == ERROR_SUCCESS) {
+            if (type == REG_DWORD_BIG_ENDIAN)
+              dword_value = base::NetToHost32(dword_value);
+            else
+              dword_value = base::ByteSwapToLE32(dword_value);
+            dict->SetIntegerWithoutPathExpansion(name, dword_value);
+            continue;
+          }
+          break;
+        }
+        case REG_NONE:
+        case REG_LINK:
+        case REG_MULTI_SZ:
+        case REG_RESOURCE_LIST:
+        case REG_FULL_RESOURCE_DESCRIPTOR:
+        case REG_RESOURCE_REQUIREMENTS_LIST:
+        case REG_QWORD_LITTLE_ENDIAN:
+          // Unsupported type, message gets logged below.
+          break;
+      }
     }
 
-    case base::Value::TYPE_INTEGER: {
-      int int_value;
-      if (base::StringToInt(value, &int_value))
-          return base::Value::CreateIntegerValue(int_value);
-      return NULL;
-    }
+    LOG(WARNING) << "Failed to read hive " << hive << " at "
+                 << root << kPathSep << name
+                 << " type " << type;
+  }
 
-    case base::Value::TYPE_DOUBLE: {
-      double double_value;
-      if (base::StringToDouble(UTF16ToUTF8(value), &double_value))
-        return base::Value::CreateDoubleValue(double_value);
-      DLOG(WARNING) << "Failed to read policy value as double: " << value;
-      return NULL;
+  // Recurse for all subkeys.
+  for (RegistryKeyIterator it(hive, root.c_str()); it.Valid(); ++it) {
+    std::string name(UTF16ToUTF8(it.Name()));
+    if (dict->HasKey(name)) {
+      DLOG(WARNING) << "Ignoring registry key because a value exists with the "
+                       "same name: " << root << kPathSep << name;
+    } else {
+      scoped_ptr<base::DictionaryValue> subdict(new base::DictionaryValue());
+      ReadRegistry(hive, root + kPathSep + it.Name(), subdict.get());
+      dict->SetWithoutPathExpansion(name, subdict.release());
     }
-
-    case base::Value::TYPE_STRING:
-      return base::Value::CreateStringValue(value);
-
-    case base::Value::TYPE_DICTIONARY:
-    case base::Value::TYPE_LIST:
-      return base::JSONReader::Read(UTF16ToUTF8(value));
-
-    case base::Value::TYPE_BINARY:
-      DLOG(WARNING) << "Cannot convert REG_SZ entry to type " << type;
-      return NULL;
   }
-  NOTREACHED();
-  return NULL;
 }
 
-// Converts an integer |value| to another |type|, if possible.
-base::Value* ConvertIntegerValue(uint32 value, base::Value::Type type) {
-  switch (type) {
-    case base::Value::TYPE_BOOLEAN:
-      return base::Value::CreateBooleanValue(value != 0);
-
-    case base::Value::TYPE_INTEGER:
-      return base::Value::CreateIntegerValue(value);
-
-    case base::Value::TYPE_DOUBLE:
-      return base::Value::CreateDoubleValue(value);
-
-    case base::Value::TYPE_NULL:
-    case base::Value::TYPE_STRING:
-    case base::Value::TYPE_BINARY:
-    case base::Value::TYPE_DICTIONARY:
-    case base::Value::TYPE_LIST:
-      DLOG(WARNING) << "Cannot convert REG_DWORD entry to type " << type;
-      return NULL;
+// Converts |value| in raw GPO representation to the internal policy value, as
+// described by |schema|. This maps the ambiguous GPO data types to the
+// internal policy value representations.
+scoped_ptr<base::Value> ConvertPolicyValue(
+    const base::Value& value,
+    const base::DictionaryValue* schema) {
+  // Figure out the type to convert to from the schema.
+  const base::Value::Type result_type(
+      GetValueTypeForSchema(schema, value.GetType()));
+
+  // If the type is good already, go with it.
+  if (value.IsType(result_type)) {
+    // Recurse for complex types if there is a schema.
+    if (schema) {
+      const base::DictionaryValue* dict = NULL;
+      const base::ListValue* list = NULL;
+      if (value.GetAsDictionary(&dict)) {
+        scoped_ptr<base::DictionaryValue> result(new base::DictionaryValue());
+        for (base::DictionaryValue::Iterator entry(*dict); entry.HasNext();
+             entry.Advance()) {
+          scoped_ptr<base::Value> converted_value(
+              ConvertPolicyValue(entry.value(),
+                                 GetSchemaFor(schema, entry.key())));
+          result->Set(entry.key(), converted_value.release());
+        }
+        return result.Pass();
+      } else if (value.GetAsList(&list)) {
+        scoped_ptr<base::ListValue> result(new base::ListValue());
+        for (base::ListValue::const_iterator entry(list->begin());
+             entry != list->end(); ++entry) {
+          scoped_ptr<base::Value> converted_value(
+              ConvertPolicyValue(**entry, GetEntry(schema, schema::kItems)));

[Joao da Silva, 2013/04/05 13:55:01]

The GetEntry() call can be outside the loop

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

Done.

+          result->Append(converted_value.release());
+        }
+        return result.Pass();
+      }
+    }
+    return make_scoped_ptr(value.DeepCopy());
   }
-  NOTREACHED();
-  return NULL;
-}
 
-// Reads a value from the registry |key| named |name| with registry type
-// |registry_type| as a value of type |type|.
-// Returns NULL if the value could not be loaded or converted.
-base::Value* ReadPolicyValue(RegKey* key,
-                             const string16& name,
-                             DWORD registry_type,
-                             base::Value::Type type) {
-  switch (registry_type) {
-    case REG_SZ: {
-      string16 value;
-      if (ReadRegistryString(key, name, &value))
-        return ConvertStringValue(value, type);
+  // Else, do some conversions to map windows registry data types to JSON types.
+  std::string string_value;
+  int int_value = 0;
+  switch (result_type) {
+    case base::Value::TYPE_NULL: {
+      return make_scoped_ptr(base::Value::CreateNullValue());
+    }
+    case base::Value::TYPE_BOOLEAN: {
+      // Accept booleans encoded as either string or integer.
+      if (value.GetAsInteger(&int_value) ||
+          (value.GetAsString(&string_value) &&
+           base::StringToInt(string_value, &int_value))) {
+        return make_scoped_ptr(Value::CreateBooleanValue(int_value != 0));
+      }
       break;
     }
-
-    case REG_DWORD: {
-      uint32 value;
-      if (ReadRegistryInteger(key, name, &value))
-        return ConvertIntegerValue(value, type);
+    case base::Value::TYPE_INTEGER: {
+      // Integers may be string-encoded.
+      if (value.GetAsString(&string_value) &&
+          base::StringToInt(string_value, &int_value)) {
+        return make_scoped_ptr(base::Value::CreateIntegerValue(int_value));
+      }
       break;
     }
-
-    default:
-      DLOG(WARNING) << "Registry type not supported for key " << name;
+    case base::Value::TYPE_DOUBLE: {
+      // Doubles may be string-encoded or integer-encoded.
+      double double_value = 0;
+      if (value.GetAsInteger(&int_value)) {
+        return make_scoped_ptr(base::Value::CreateDoubleValue(int_value));
+      } else if (value.GetAsString(&string_value) &&
+                 base::StringToDouble(string_value, &double_value)) {
+        return make_scoped_ptr(base::Value::CreateDoubleValue(double_value));
+      }
       break;
-  }
-  return NULL;
-}
-
-// Forward declaration for ReadComponentListValue().
-base::DictionaryValue* ReadComponentDictionaryValue(
-    HKEY hive,
-    const string16& path,
-    const base::DictionaryValue* schema);
-
-// Loads the list at |path| in the given |hive|. |schema| is a JSON schema
-// (http://json-schema.org/) that describes the expected type of the list.
-// Ownership of the result is transferred to the caller.
-base::ListValue* ReadComponentListValue(HKEY hive,
-                                        const string16& path,
-                                        const base::DictionaryValue* schema) {
-  // The sub-elements are indexed from 1 to N. They can be represented as
-  // registry values or registry keys though; use |schema| first to try to
-  // determine the right type, and if that fails default to STRING.
-
-  RegKey key(hive, path.c_str(), KEY_READ);
-  if (!key.Valid())
-    return NULL;
-
-  // Get the schema for list items.
-  schema = GetEntry(schema, schema::kItems);
-  base::Value::Type type = GetType(schema, base::Value::TYPE_STRING);
-  base::ListValue* list = new base::ListValue();
-  for (int i = 1; ; ++i) {
-    string16 name = base::IntToString16(i);
-    base::Value* value = NULL;
-    if (type == base::Value::TYPE_DICTIONARY) {
-      value =
-          ReadComponentDictionaryValue(hive, path + kPathSep + name, schema);
-    } else if (type == base::Value::TYPE_LIST) {
-      value = ReadComponentListValue(hive, path + kPathSep + name, schema);
-    } else {
-      DWORD reg_type;
-      key.ReadValue(name.c_str(), NULL, NULL, &reg_type);
-      if (reg_type != REG_NONE)
-        value = ReadPolicyValue(&key, name, reg_type, type);
     }
-    if (value)
-      list->Append(value);
-    else
+    case base::Value::TYPE_LIST: {
+      // Lists are encoded as subkeys with numbered value in the registry.
+      const base::DictionaryValue* dict = NULL;
+      if (value.GetAsDictionary(&dict)) {
+        scoped_ptr<base::ListValue> result(new base::ListValue());
+        for (int i = 1; ; ++i) {
+          const base::Value* entry = NULL;
+          if (!dict->Get(base::IntToString(i), &entry))
+            break;
+          scoped_ptr<base::Value> converted_value(
+              ConvertPolicyValue(*entry, GetEntry(schema, schema::kItems)));

[Joao da Silva, 2013/04/05 13:55:01]

GetEntry() can be outside the loop

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

Done.

+          result->Append(converted_value.release());
+        }
+        return result.Pass();
+      }
+      // Fall through in order to accept lists encoded as JSON strings.
+    }
+    case base::Value::TYPE_DICTIONARY: {
+      // Dictionaries may be encoded as JSON strings.
+      if (value.GetAsString(&string_value)) {
+        scoped_ptr<base::Value> result(base::JSONReader::Read(string_value));
+        if (result && result->IsType(result_type))
+          return result.Pass();
+      }
+      break;
+    }
+    case base::Value::TYPE_STRING:
+    case base::Value::TYPE_BINARY:
+      // No conversion possible.
       break;
   }
-  return list;
-}
 
-// Loads the dictionary at |path| in the given |hive|. |schema| is a JSON
-// schema (http://json-schema.org/) that describes the expected types for the
-// dictionary entries. When the type for a certain entry isn't described in the
-// schema, a default conversion takes place. |schema| can be NULL.
-// Ownership of the result is transferred to the caller.
-base::DictionaryValue* ReadComponentDictionaryValue(
-    HKEY hive,
-    const string16& path,
-    const base::DictionaryValue* schema) {
-  // A "value" in the registry is like a file in a filesystem, and a "key" is
-  // like a directory, that contains other "values" and "keys".
-  // Unfortunately it is possible to have a name both as a "value" and a "key".
-  // In those cases, the sub "key" will be ignored; this choice is arbitrary.
-
-  // First iterate over all the "values" in |path| and convert them; then
-  // recurse into each "key" in |path| and convert them as dictionaries.
+  LOG(WARNING) << "Failed to convert " << value.GetType()
+                << " to " << result_type;

[Joao da Silva, 2013/04/05 13:55:01]

nit: indent

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

Done.

+  return make_scoped_ptr(base::Value::CreateNullValue());
+}
 
-  RegKey key(hive, path.c_str(), KEY_READ);
-  if (!key.Valid())
-    return NULL;
+// Parses |gpo_dict| according to |schema| and writes the resulting policy
+// settings to |policy| for the given |scope| and |level|.
+void ParsePolicy(const base::DictionaryValue* gpo_dict,
+                 PolicyLevel level,
+                 PolicyScope scope,
+                 const base::DictionaryValue* schema,
+                 PolicyMap* policy) {
+  if (!gpo_dict)
+    return;
+
+  scoped_ptr<base::Value> policy_value(ConvertPolicyValue(*gpo_dict, schema));
+  const base::DictionaryValue* policy_dict = NULL;
+  if (!policy_value->GetAsDictionary(&policy_dict) || !policy_dict) {
+    LOG(WARNING) << "Root policy object is not a dictionary!";
+    return;
+  }
 
-  base::DictionaryValue* dict = new base::DictionaryValue();
-  for (RegistryValueIterator it(hive, path.c_str()); it.Valid(); ++it) {
-    string16 name16(it.Name());
-    std::string name(UTF16ToUTF8(name16));
-    const base::DictionaryValue* sub_schema = GetSchemaFor(schema, name);
-    base::Value::Type type = GetType(sub_schema, GetDefaultFor(it.Type()));
-    base::Value* value = ReadPolicyValue(&key, name16, it.Type(), type);
-    if (value)
-      dict->Set(name, value);
+  for (base::DictionaryValue::Iterator it(*policy_dict); it.HasNext();
+       it.Advance()) {
+    policy->Set(it.key(), level, scope, it.value().DeepCopy());
   }

[Joao da Silva, 2013/04/05 13:55:01]

policy->LoadFrom(policy_dict, level, scope)

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

Done.

+}
 
-  for (RegistryKeyIterator it(hive, path.c_str()); it.Valid(); ++it) {
-    string16 name16(it.Name());
-    std::string name(UTF16ToUTF8(name16));
-    if (dict->HasKey(name)) {
-      DLOG(WARNING) << "Ignoring registry key because a value exists with the "
-                       "same name: " << path << kPathSep << name;
+// Queries Windows for applied group policy and write the result to |policy|.
+// This is the preferred way to obtain GPO data, there are reports of abuse
+// of the registry GPO keys by 3rd-party software.
+bool ReadPolicyFromGPO(PolicyScope scope, base::DictionaryValue* policy) {
+  PGROUP_POLICY_OBJECT policy_object_list_pointer = NULL;
+  DWORD flags = scope == POLICY_SCOPE_MACHINE ? GPO_LIST_FLAG_MACHINE : 0;
+  if (GetAppliedGPOList(flags, NULL, NULL, &kRegistrySettingsCSEGUID,
+                        &policy_object_list_pointer)) {
+    PLOG(ERROR) << "GetAppliedGPOList for scope " << scope << " failed";
+    return false;

[Joao da Silva, 2013/04/05 13:55:01]

What happens in a machine without any policy? Does it fail here, or does it
succeed with an empty policy_object_list?

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

Yes, I saw that happening in a pristine VM.

+  }
+  scoped_ptr<GROUP_POLICY_OBJECT, FreeGPOListDeleter> policy_object_list(
+      policy_object_list_pointer);
+
+  base::DictionaryValue regular_policy;
+  base::DictionaryValue forced_policy;
+  for (GROUP_POLICY_OBJECT* policy_object = policy_object_list.get();
+       policy_object; policy_object = policy_object->pNext) {
+/*
+    LOG(ERROR) << "GPO object: " << std::endl
+               << "options " << policy_object->dwOptions << std::endl
+               << "version " << policy_object->dwVersion << std::endl
+               << "dspath " << policy_object->lpDSPath << std::endl
+               << "lpFileSysPath " << policy_object->lpFileSysPath << std::endl
+               << "lpDisplayName " << policy_object->lpDisplayName << std::endl
+               << "GPOName " << policy_object->szGPOName << std::endl
+               << "GPOLink " << policy_object->GPOLink << std::endl
+               << "lParam " << policy_object->lParam << std::endl
+               << "next " << policy_object->pNext << std::endl
+               << "prev " << policy_object->pPrev << std::endl
+               << "lpExtensions " << policy_object->lpExtensions << std::endl
+               << "lParam2 " << policy_object->lParam2 << std::endl
+               << "lpLink " << policy_object->lpLink << std::endl;

[Joao da Silva, 2013/04/05 13:55:01]

You've been having some fun eh :-)

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

Done.

+*/
+
+    if (policy_object->dwOptions & GPO_FLAG_DISABLE)

[Joao da Silva, 2013/04/05 13:55:01]

It's not clear to me if this flag is set for boolean policies that are set to
"disabled" in the GPO editor, did you test?

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

This refers to the entire GroupPolicyObject, not individual policy settings.

       continue;
+
+    if (PathIsUNC(policy_object->lpFileSysPath)) {

[Joao da Silva, 2013/04/05 13:55:01]

Can policy_object->lpDSPath have more policy that isn't in lpFileSysPath? Maybe
this test should be

if (PathIsUNC(policy_object->lpFileSysPath) || !policy_object->lpDSPath.empty())

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

I've actually looked at the DS portion, there's only meta data there.

+      // UNC path: Assume this is an AD-managed machine, which updates the
+      // registry via GPO's standard registry CSE periodically. Fall back to
+      // reading from the registry in this case.
+      return false;
     }
 
-    const base::DictionaryValue* sub_schema = GetSchemaFor(schema, name);
-    base::Value::Type type = GetType(sub_schema, base::Value::TYPE_DICTIONARY);
-    base::Value* value = NULL;
-    const string16 sub_path = path + kPathSep + name16;
-    if (type == base::Value::TYPE_DICTIONARY) {
-      value = ReadComponentDictionaryValue(hive, sub_path, sub_schema);
-    } else if (type == base::Value::TYPE_LIST) {
-      value = ReadComponentListValue(hive, sub_path, sub_schema);
+    base::FilePath preg_file_path(
+        base::FilePath(policy_object->lpFileSysPath).Append(kPRegFileName));
+    if (policy_object->dwOptions & GPO_FLAG_FORCE) {
+      base::DictionaryValue new_forced_policy;
+      if (!preg_parser::ReadFile(preg_file_path, &new_forced_policy))

[Joao da Silva, 2013/04/05 13:55:01]

This is going to read all the policies for all the managed keys; how about
extending ReadFile() to only load stuff within a path prefix?

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

I had done that already, but not updated the uploaded CL yet :)

+        return false;
+
+      // Merge with existing forced policy, giving precedence to the existing
+      // forced policy.
+      new_forced_policy.MergeDictionary(&forced_policy);
+      forced_policy.Swap(&new_forced_policy);
     } else {
-      DLOG(WARNING) << "Can't read a simple type in registry key at " << path;
+      if (!preg_parser::ReadFile(preg_file_path, &regular_policy))
+        return false;
     }
-    if (value)
-      dict->Set(name, value);
   }
 
-  return dict;
-}
+  // Construct the resulting policy data dictionary.
+  base::DictionaryValue gpo_policy;
+  gpo_policy.MergeDictionary(&regular_policy);

[Joao da Silva, 2013/04/05 13:55:01]

Swap?

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

Done.

+  gpo_policy.MergeDictionary(&forced_policy);
 
-// Reads a JSON schema from the given |registry_value|, at the given
-// |registry_key| in |hive|. |registry_value| must be a string (REG_SZ), and
-// is decoded as JSON data. Returns NULL on failure. Ownership is transferred
-// to the caller.
-base::DictionaryValue* ReadRegistrySchema(HKEY hive,
-                                          const string16& registry_key,
-                                          const string16& registry_value) {
-  RegKey key(hive, registry_key.c_str(), KEY_READ);
-  string16 schema;
-  if (!ReadRegistryString(&key, registry_value, &schema))
-    return NULL;
-  // A JSON schema is represented in JSON too.
-  scoped_ptr<base::Value> value(base::JSONReader::Read(UTF16ToUTF8(schema)));
-  if (!value.get())
-    return NULL;
-  base::DictionaryValue* dict = NULL;
-  if (!value->GetAsDictionary(&dict))
-    return NULL;
-  // The top-level entry must be an object, and each of its properties maps
-  // a policy name to its schema.
-  if (GetType(dict, base::Value::TYPE_DICTIONARY) !=
-      base::Value::TYPE_DICTIONARY) {
-    DLOG(WARNING) << "schema top-level type isn't \"object\"";
-    return NULL;
-  }
-  value.release();
-  return dict;
+  // Extract the Chrome-specific part.
+  std::string path;
+  ReplaceChars(UTF16ToUTF8(kRegistryChromePolicyKey), "\\", ".", &path);
+  base::DictionaryValue* chrome_policy;
+  if (gpo_policy.GetDictionary(path, &chrome_policy))
+    policy->Swap(chrome_policy);
+
+  return true;
 }
 
 }  // namespace
@@ -410,155 +463,153 @@ PolicyLoaderWin::~PolicyLoaderWin() {
 }
 
 void PolicyLoaderWin::InitOnFile() {
+  // Build the Chrome policy schema.
+  scoped_ptr<base::DictionaryValue> properties(new base::DictionaryValue());
+  for (const PolicyDefinitionList::Entry* e = policy_list_->begin;
+       e != policy_list_->end; ++e) {
+    const std::string schema_type = GetSchemaTypeForValueType(e->value_type);
+    scoped_ptr<base::DictionaryValue> entry_schema(new base::DictionaryValue());
+    entry_schema->SetStringWithoutPathExpansion(json_schema_constants::kType,
+                                                schema_type);
+
+    if (e->value_type == base::Value::TYPE_LIST) {
+      scoped_ptr<base::DictionaryValue> items_schema(
+          new base::DictionaryValue());
+      items_schema->SetStringWithoutPathExpansion(
+          json_schema_constants::kType, json_schema_constants::kString);
+      entry_schema->SetWithoutPathExpansion(json_schema_constants::kItems,
+                                            items_schema.release());
+    }
+    properties->SetWithoutPathExpansion(e->name, entry_schema.release());
+  }
+  chrome_policy_schema_.SetStringWithoutPathExpansion(
+      json_schema_constants::kType, json_schema_constants::kObject);
+  chrome_policy_schema_.SetWithoutPathExpansion(
+      json_schema_constants::kProperties, properties.release());

[Joao da Silva, 2013/04/05 13:55:01]

Using a DictionaryBuilder here may make this cleaner.

[Mattias Nissler (ping if slow), 2013/04/09 22:43:38]

DictionaryBuilder is still in chrome/common/extensions, so I won't use it :)

+
   is_initialized_ = true;
   SetupWatches();
 }
 
 scoped_ptr<PolicyBundle> PolicyLoaderWin::Load() {
-  scoped_ptr<PolicyBundle> bundle(new PolicyBundle());
-  LoadChromePolicy(
-      &bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())));
-  Load3rdPartyPolicies(bundle.get());
-  return bundle.Pass();
-}
-
-void PolicyLoaderWin::LoadChromePolicy(PolicyMap* chrome_policies) {
   // Reset the watches BEFORE reading the individual policies to avoid
   // missing a change notification.
   if (is_initialized_)
     SetupWatches();
 
-  // |kKeyPaths| is in decreasing order of priority.
+  // Policy scope and corresponding hive.
   static const struct {
-    const wchar_t* path;
-    PolicyLevel level;
-  } kKeyPaths[] = {
-    { kRegistryMandatorySubKey,   POLICY_LEVEL_MANDATORY    },
-    { kRegistryRecommendedSubKey, POLICY_LEVEL_RECOMMENDED  },
+    PolicyScope scope;
+    HKEY hive;
+  } kScopes[] = {
+    { POLICY_SCOPE_MACHINE, HKEY_LOCAL_MACHINE },
+    { POLICY_SCOPE_USER,    HKEY_CURRENT_USER  },
   };
 
-  // Lookup at the mandatory path for both user and machine policies first, and
-  // then at the recommended path.
-  for (size_t k = 0; k < arraysize(kKeyPaths); ++k) {
-    for (size_t h = 0; h < arraysize(kHives); ++h) {
-      // Iterate over keys and values at this hive and path.
-      HKEY hive = kHives[h].hive;
-      string16 path(kKeyPaths[k].path);
-      RegKey key;
-      if (key.Open(hive, path.c_str(), KEY_READ) != ERROR_SUCCESS ||
-          !key.Valid()) {
-        continue;
-      }
-
-      // Iterate over values for most policies.
-      for (RegistryValueIterator it(hive, path.c_str()); it.Valid(); ++it) {
-        std::string name(UTF16ToUTF8(it.Name()));
-        // Skip if a higher-priority policy value was already inserted, or
-        // if this is the default value (empty string).
-        if (chrome_policies->Get(name) || name.empty())
-          continue;
-        // Get the expected policy type, if this is a known policy.
-        base::Value::Type type = GetDefaultFor(it.Type());
-        for (const PolicyDefinitionList::Entry* e = policy_list_->begin;
-             e != policy_list_->end; ++e) {
-          if (name == e->name) {
-            type = e->value_type;
-            break;
-          }
-        }
-        base::Value* value = ReadPolicyValue(&key, it.Name(), it.Type(), type);
-        if (!value)
-          value = base::Value::CreateNullValue();
-        chrome_policies->Set(name, kKeyPaths[k].level, kHives[h].scope, value);
-      }
-
-      // Iterate over keys for policies of type string-list.
-      for (RegistryKeyIterator it(hive, path.c_str()); it.Valid(); ++it) {
-        std::string name(UTF16ToUTF8(it.Name()));
-        // Skip if a higher-priority policy value was already inserted, or
-        // if this is the 3rd party policy subkey.
-        const string16 kThirdParty16(kThirdParty);
-        if (chrome_policies->Get(name) || it.Name() == kThirdParty16)
-          continue;
-        string16 list_path = path + kPathSep + it.Name();
-        RegKey key;
-        if (key.Open(hive, list_path.c_str(), KEY_READ) != ERROR_SUCCESS ||
-            !key.Valid()) {
-          continue;
-        }
-        base::ListValue* result = new base::ListValue();
-        string16 value;
-        int index = 0;
-        while (ReadRegistryString(&key, base::IntToString16(++index), &value))
-          result->Append(base::Value::CreateStringValue(value));
-        chrome_policies->Set(name, kKeyPaths[k].level, kHives[h].scope, result);
-      }
-    }
+  // Load policy data for the different scopes/levels and merge them.
+  scoped_ptr<PolicyBundle> bundle(new PolicyBundle());
+  PolicyMap* chrome_policy =
+      &bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()));
+  for (size_t i = 0; i < arraysize(kScopes); ++i) {
+    PolicyScope scope = kScopes[i].scope;
+    base::DictionaryValue gpo_dict;
+    if (!ReadPolicyFromGPO(scope, &gpo_dict))
+      ReadRegistry(kScopes[i].hive, kRegistryChromePolicyKey, &gpo_dict);
+
+    // Remove special-cased entries from the GPO dictionary.
+    base::DictionaryValue* temp_dict = NULL;
+    scoped_ptr<base::DictionaryValue> recommended_dict(
+        RemoveDict(&gpo_dict, kKeyRecommended));
+    scoped_ptr<base::DictionaryValue> third_party_dict(
+        RemoveDict(&gpo_dict, kKeyThirdParty));
+
+    // Load Chrome policy.
+    LoadChromePolicy(&gpo_dict, POLICY_LEVEL_MANDATORY, scope, chrome_policy);
+    LoadChromePolicy(recommended_dict.get(), POLICY_LEVEL_RECOMMENDED, scope,
+                     chrome_policy);
+
+    // Load 3rd-party policy.
+    if (third_party_dict)
+      Load3rdPartyPolicy(third_party_dict.get(), scope, bundle.get());
   }
+
+  return bundle.Pass();
 }
 
-void PolicyLoaderWin::Load3rdPartyPolicies(PolicyBundle* bundle) {
-  // Each 3rd party namespace can have policies on both HKLM and HKCU. They
-  // should be merged, giving priority to HKLM for policies with the same name.
+void PolicyLoaderWin::LoadChromePolicy(const base::DictionaryValue* gpo_dict,
+                                       PolicyLevel level,
+                                       PolicyScope scope,
+                                       PolicyMap* chrome_policy_map) {
+  PolicyMap policy;
+  ParsePolicy(gpo_dict, level, scope, &chrome_policy_schema_, &policy);
+  chrome_policy_map->MergeFrom(policy);
+}
 
-  // Map of known domain name to their enum values.
+void PolicyLoaderWin::Load3rdPartyPolicy(
+    const DictionaryValue* gpo_dict,
+    PolicyScope scope,
+    PolicyBundle* bundle) {
+  // Map of known 3rd party policy domain name to their enum values.
   static const struct {
     const char* name;
     PolicyDomain domain;
-  } kDomains[] = {
-    { "extensions",   POLICY_DOMAIN_EXTENSIONS },
+  } k3rdPartyDomains[] = {
+    { "extensions", POLICY_DOMAIN_EXTENSIONS },
   };
 
-  // Map of policy paths to their corresponding policy level, in decreasing
-  // order of priority.
+  // Policy level and corresponding path.
   static const struct {
-    const char* path;
     PolicyLevel level;
-  } kKeyPaths[] = {
-    { "policy",       POLICY_LEVEL_MANDATORY },
-    { "recommended",  POLICY_LEVEL_RECOMMENDED },
+    const char* path;
+  } kLevels[] = {
+    { POLICY_LEVEL_MANDATORY,   kKeyMandatory   },
+    { POLICY_LEVEL_RECOMMENDED, kKeyRecommended },
   };
 
-  // Path where policies for components are stored.
-  const string16 kPathPrefix = string16(kRegistryMandatorySubKey) + kPathSep +
-                               kThirdParty + kPathSep;
-
-  for (size_t h = 0; h < arraysize(kHives); ++h) {
-    HKEY hkey = kHives[h].hive;
-
-    for (size_t d = 0; d < arraysize(kDomains); ++d) {
-      // Each subkey under this domain is a component of that domain.
-      // |domain_path| == SOFTWARE\Policies\Chromium\3rdparty\<domain>
-      string16 domain_path = kPathPrefix + ASCIIToUTF16(kDomains[d].name);
-
-      for (RegistryKeyIterator domain_iterator(hkey, domain_path.c_str());
-           domain_iterator.Valid(); ++domain_iterator) {
-        string16 component(domain_iterator.Name());
-        string16 component_path = domain_path + kPathSep + component;
-
-        // Load the schema for this component's policy, if present.
-        scoped_ptr<base::DictionaryValue> schema(
-            ReadRegistrySchema(hkey, component_path, kSchema));
-
-        for (size_t k = 0; k < arraysize(kKeyPaths); ++k) {
-          string16 path =
-              component_path + kPathSep + ASCIIToUTF16(kKeyPaths[k].path);
-
-          scoped_ptr<base::DictionaryValue> dictionary(
-              ReadComponentDictionaryValue(hkey, path, schema.get()));
-          if (dictionary.get()) {
-            PolicyMap policies;
-            policies.LoadFrom(
-                dictionary.get(), kKeyPaths[k].level, kHives[h].scope);
-            // LoadFrom() overwrites any existing values. Use a temporary map
-            // and then use MergeFrom(), that only overwrites values with lower
-            // priority.
-            bundle->Get(PolicyNamespace(kDomains[d].domain,
-                                        UTF16ToUTF8(component)))
-                .MergeFrom(policies);
-          }
+  for (size_t i = 0; i < arraysize(k3rdPartyDomains); i++) {
+    const char* name = k3rdPartyDomains[i].name;
+    const PolicyDomain domain = k3rdPartyDomains[i].domain;
+    const base::DictionaryValue* domain_dict = NULL;
+    if (!gpo_dict->GetDictionaryWithoutPathExpansion(name, &domain_dict) ||
+        !domain_dict) {
+      continue;
+    }
+
+    for (base::DictionaryValue::Iterator component(*domain_dict);
+         component.HasNext(); component.Advance()) {
+      const base::DictionaryValue* component_dict = NULL;
+      if (!component.value().GetAsDictionary(&component_dict) ||
+          !component_dict) {
+        continue;
+      }
+
+      // Load the schema.
+      scoped_ptr<base::Value> schema;
+      const base::DictionaryValue* schema_dict = NULL;
+      std::string schema_json;
+      if (component_dict->GetStringWithoutPathExpansion(kKeySchema,
+                                                        &schema_json)) {
+        schema.reset(base::JSONReader::Read(schema_json));
+        if (!schema || !schema->GetAsDictionary(&schema_dict)) {
+          LOG(WARNING) << "Failed to parse 3rd-part policy schema for "
+                       << domain << "/" << component.key();
         }
       }
+
+      // Parse policy.
+      for (size_t j = 0; j < arraysize(kLevels); j++) {
+        const base::DictionaryValue* policy_dict = NULL;
+        if (!component_dict->GetDictionaryWithoutPathExpansion(
+                kLevels[j].path, &policy_dict) ||
+            !policy_dict) {
+          continue;
+        }
+
+        PolicyMap policy;
+        ParsePolicy(policy_dict, kLevels[j].level, scope, schema_dict, &policy);
+        PolicyNamespace policy_namespace(domain, component.key());
+        bundle->Get(policy_namespace).MergeFrom(policy);
+      }
     }
   }
 }