Add parser for PReg files.

chrome/browser/policy/preg_parser_win.cc

+// Copyright (c) 2013 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 "chrome/browser/policy/preg_parser_win.h"
+
+#include <windows.h>
+
+#include "base/basictypes.h"
+#include "base/files/file_path.h"
+#include "base/files/memory_mapped_file.h"
+#include "base/logging.h"
+#include "base/stl_util.h"
+#include "base/string16.h"
+#include "base/string_util.h"
+#include "base/sys_byteorder.h"
+#include "base/utf_string_conversions.h"
+#include "base/values.h"
+
+namespace policy {
+namespace preg_parser {
+
+// The magic header in PReg files: ASCII "PReg" + version (0x0001).
+const char kPolicyRegistryFileHeader[] = "PReg\x01\x00\x00\x00";
+
+// Constants for PReg file delimiters.
+#if defined(ARCH_CPU_LITTLE_ENDIAN)
+#define BYTE_SWAP_TO_LE_16(x) (x)
+#else
+#define BYTE_SWAP_TO_LE_16(x) ((((x) & 0xff00) >> 8) | (((x) & 0x00ff) << 8))
+#endif
+const char16 kDelimBracketOpen = BYTE_SWAP_TO_LE_16(L'[');
+const char16 kDelimBracketClose = BYTE_SWAP_TO_LE_16(L']');
+const char16 kDelimSemicolon = BYTE_SWAP_TO_LE_16(L';');
+#undef BYTE_SWAP_TO_LE_16
+
+// Magic strings for the PReg value field to trigger special actions.
+const char kActionTriggerPrefix[] = "**";
+const char kActionTriggerDeleteValues[] = "deletevalues";
+const char kActionTriggerDel[] = "del.";
+const char kActionTriggerDelVals[] = "delvals";
+const char kActionTriggerDeleteKeys[] = "deletekeys";
+const char kActionTriggerSecureKey[] = "securekey";
+const char kActionTriggerSoft[] = "soft";
+
+// Swaps all characters of a string16 from LE to host byte order.
+void String16LEToHost(string16* str) {
+  for (string16::iterator c(str->begin()); c != str->end(); ++c)
+    *c = base::ByteSwapToLE16(*c);
+}
+
+// Reads a fixed-size field from a PReg file.
+bool ReadFieldBinary(const char16** cursor,
+                     const char16* end,
+                     int size,
+                     uint8* data) {
+  const uint8* start_bytes = reinterpret_cast<const uint8*>(*cursor);
+  const uint8* end_bytes = start_bytes + size;
+  if (end_bytes >= reinterpret_cast<const uint8*>(end))
+    return false;
+  std::copy(start_bytes, end_bytes, data);
+  *cursor = reinterpret_cast<const char16*>(end_bytes);

[Joao da Silva, 2013/04/05 12:51:33]

Unfortunately the documentation doesn't mention what happens wrt alignment here,
when size is odd; I guess this is fine.

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

Standard says this is implementation-defined. I've changed the parsing to work
with uint8 now to be on the safe side. Who knows, at some point this will run on
ARM? ;)

+  return true;
+}
+
+bool ReadField32(const char16** cursor, const char16* end, uint32* data) {
+  uint32 value = 0;
+  if (!ReadFieldBinary(cursor, end, sizeof(uint32),
+                       reinterpret_cast<uint8*>(&value))) {
+    return false;
+  }
+  *data = base::ByteSwapToLE32(value);
+  return true;
+}
+
+// Reads a string field from a  file.
+bool ReadFieldString(const char16** cursor, const char16* end, string16* str) {
+  const char16* key_start = *cursor;
+  for (; *cursor < end; ++*cursor) {
+    if (**cursor == 0x0000) {
+      str->assign(key_start, *cursor);
+      String16LEToHost(str);
+      ++*cursor;
+      return true;
+    }
+  }
+  return false;
+}
+
+std::string DecodePRegStringValue(const std::vector<uint8>& data) {
+  // Subtract one to account for the trailing null character.
+  const size_t len = std::max(0U, (data.size() / sizeof(char16)) - 1);

[Joao da Silva, 2013/04/05 12:51:33]

If data.size() is 0 then the 2nd arg to max() becomes SIZE_MAX.

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

Done.

+  const char16* chars = reinterpret_cast<const char16*>(vector_as_array(&data));
+  return UTF16ToUTF8(string16(chars, len));
+}
+
+// Decodes a value from a PReg file given as a uint8 vector.
+bool DecodePRegValue(uint32 type,
+                     const std::vector<uint8>& data,
+                     scoped_ptr<base::Value>* value) {
+  switch (type) {
+    case REG_SZ:
+    case REG_EXPAND_SZ:
+      value->reset(base::Value::CreateStringValue(DecodePRegStringValue(data)));
+      return true;
+    case REG_DWORD_LITTLE_ENDIAN:
+    case REG_DWORD_BIG_ENDIAN:
+      if (data.size() == sizeof(uint32)) {
+        uint32 val = *reinterpret_cast<const uint32*>(vector_as_array(&data));
+        if (type == REG_DWORD_BIG_ENDIAN)
+          val = base::NetToHost32(val);
+        else
+          val = base::ByteSwapToLE32(val);
+        value->reset(base::Value::CreateIntegerValue(static_cast<int>(val)));
+        return true;
+      } else {
+        LOG(ERROR) << "Bad data size " << data.size();
+      }
+      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:
+    default:

[Joao da Silva, 2013/04/05 12:51:33]

Why have some cases and a default, and not just default?

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

To document that we're purposely not implementing those, and that this is the
list that I have considered. If some windows header has additional ones, we'll
know that I didn't have them on the radar when writing this.

+      LOG(ERROR) << "Unsupported registry data type " << type;
+  }
+
+  return false;
+}
+
+// Adds the record data passed via parameters to |dict| in case the data is
+// relevant policy for Chromium.
+void HandleRecord(const string16& key_name,
+                  const string16& value,
+                  uint32 type,
+                  const std::vector<uint8>& data,
+                  base::DictionaryValue* dict) {
+  // Locate/create the dictionary to place the value in.
+  std::vector<std::string> path;
+  Tokenize(UTF16ToUTF8(key_name), "\\", &path);
+  for (std::vector<std::string>::const_iterator entry(path.begin());
+       entry != path.end(); ++entry) {
+    base::DictionaryValue* subdict = NULL;
+    if (!dict->GetDictionaryWithoutPathExpansion(*entry, &subdict) ||
+        !subdict) {
+      subdict = new DictionaryValue();
+      dict->SetWithoutPathExpansion(*entry, subdict);
+    }
+    dict = subdict;
+  }
+
+  if (value.empty())
+    return;
+
+  std::string value_name(UTF16ToUTF8(value));
+  if (!StartsWithASCII(value_name, kActionTriggerPrefix, true)) {
+    scoped_ptr<base::Value> value;
+    if (DecodePRegValue(type, data, &value))
+      dict->Set(value_name, value.release());
+    return;
+  }
+
+  std::string action_trigger(StringToLowerASCII(value_name.substr(
+      arraysize(kActionTriggerPrefix) - 1)));
+  if (action_trigger == kActionTriggerDeleteValues ||
+      StartsWithASCII(kActionTriggerDeleteKeys, action_trigger, true)) {

[Joao da Silva, 2013/04/05 12:51:33]

The first 2 args to StartsWithASCII are swapped

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

Good catch. Done.

+    std::vector<std::string> keys;
+    Tokenize(DecodePRegStringValue(data), ";", &keys);
+    for (std::vector<std::string>::const_iterator key(keys.begin());
+         key != keys.end(); ++key) {
+      dict->RemoveWithoutPathExpansion(*key, NULL);
+    }
+  } else if (StartsWithASCII(action_trigger, kActionTriggerDel, true)) {
+    dict->RemoveWithoutPathExpansion(
+        value_name.substr(arraysize(kActionTriggerPrefix) - 1 +

[pastarmovj, 2013/04/05 08:41:43]

Isn't here one -1 too much? I suspect this worked since the last char is a .
maybe?

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

Note that the -1 here is just for taking the terminating '\0' character into
account. But let's see, the trigger might be:

**del.foobar

where:
  arraysize(kActionTriggerPrefix) = 3
  arraysize(kActionTriggerDel) = 5

so we call value_name.substr(3 - 1 + 5 - 1), i.e. value.substr(6), which is
"foobar".

Seems correct to me?

+                          arraysize(kActionTriggerDel) - 1),
+        NULL);
+  } else if (StartsWithASCII(action_trigger, kActionTriggerDelVals, true)) {
+    dict->Clear();

[Joao da Silva, 2013/04/05 12:51:33]

The documentation of DelVals says that it deletes all the values, but this also
deletes all the keys.

Keys in |dict| mapping to another DictionaryValue should be kept.

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

Interesting edge case :) Fixed.

+  } else if (StartsWithASCII(action_trigger, kActionTriggerSecureKey, true) ||
+             StartsWithASCII(action_trigger, kActionTriggerSoft, true)) {

[Joao da Silva, 2013/04/05 12:51:33]

Where did you find about "Soft"? The linked document doesn't describe that.

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

Apparently Microsoft used this at some point. I found code on the net that
checks for this, so I figured I'd be liberal in what I expect.

+    // Doesn't affect values.
+  } else {
+    LOG(ERROR) << "Bad action trigger " << value_name;
+  }
+}
+
+bool ReadFile(const base::FilePath& file_path,
+              base::DictionaryValue* dict) {
+  base::MemoryMappedFile mapped_file;
+
+  if (!mapped_file.Initialize(file_path) || !mapped_file.IsValid()) {
+    PLOG(ERROR) << "Failed to map " << file_path.value();
+    return false;
+  }
+
+  // Check the header.
+  const int kHeaderSize = arraysize(kPolicyRegistryFileHeader) - 1;
+  if (mapped_file.length() < kHeaderSize ||
+      memcmp(kPolicyRegistryFileHeader, mapped_file.data(), kHeaderSize) != 0) {
+    LOG(ERROR) << "Bad policy file " << file_path.value();
+    return false;
+  }
+
+  // Parse file contents, which is UCS-2 and little-endian. The latter I
+  // couldn't find documentation on, but the example I saw were all
+  // little-endian. It'd be interesting to check on big-endian hardware.
+  //
+  // The file format is documented here:
+  // http://msdn.microsoft.com/en-us/library/windows/desktop/aa374407(v=vs.85).aspx

[pastarmovj, 2013/04/05 08:41:43]

you can skip microsoft. and the address is still valid - msdn.com exists.

[Joao da Silva, 2013/04/05 12:51:33]

Suggestion: point to the documentation at the top of the file

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

That redirects though, and the URL I have seems to be the canonical one, so I'd
rather keep it.

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

Done.

+  const char16* cursor = reinterpret_cast<const char16*>(
+      mapped_file.data() + kHeaderSize);
+  const char16* end = reinterpret_cast<const char16*>(
+      mapped_file.data() + mapped_file.length());

[Joao da Silva, 2013/04/05 12:51:33]

What happens if the file ends in a byte i.e. half a char16?

I guess if the alignment of |end| isn't right then the file should be rejected.

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

Fixed.

+  bool parse_error = true;
+  while (cursor < end) {
+    if (*cursor++ != kDelimBracketOpen)
+      break;
+
+    // Read the record fields.
+    string16 key_name;
+    string16 value;
+    uint32 type = 0;
+    uint32 size = 0;
+    std::vector<uint8> data;
+
+    if (!ReadFieldString(&cursor, end, &key_name))
+      break;
+
+    if (*cursor == kDelimSemicolon) {
+      ++cursor;
+      if (!ReadFieldString(&cursor, end, &value))
+        break;
+    }
+
+    if (*cursor == kDelimSemicolon) {
+      ++cursor;
+      if (!ReadField32(&cursor, end, &type))
+        break;
+    }
+
+    if (*cursor == kDelimSemicolon) {
+      ++cursor;
+      if (!ReadField32(&cursor, end, &size))
+        break;
+    }
+
+    if (*cursor == kDelimSemicolon) {
+      ++cursor;
+      data.resize(size);

[Joao da Silva, 2013/04/05 12:51:33]

We should impose a size limit, otherwise borked files will prevent chrome from
running.

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

Done.

+      if (!ReadFieldBinary(&cursor, end, size, vector_as_array(&data)))
+        break;
+    }
+
+    if (*cursor != kDelimBracketClose)
+      break;
+    ++cursor;
+
+    HandleRecord(key_name, value, type, data, dict);
+
+    if (cursor == end) {
+      parse_error = false;
+      break;

[pastarmovj, 2013/04/05 08:41:43]

Why not directly return true here?

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

Good idea, done.

+    }
+  }
+
+  if (parse_error) {

[pastarmovj, 2013/04/05 08:41:43]

...then you can simply assume you encountered an error if you get here at all.

[Mattias Nissler (ping if slow), 2013/04/05 18:12:50]

Done.

+    LOG(ERROR) << "Error parsing " << file_path.value() << " at offset "
+               << reinterpret_cast<const uint8*>(cursor) - mapped_file.data();
+  }
+
+  return !parse_error;
+}
+
+}  // namespace preg_parser
+}  // namespace policy