libbrillo: policy: Increase resilience for policy read
The policy files will be stored with indexes as suffix to improve
resilience to disk corruptions. This change makes it possible to
read the policy data from new files and validate them.
Design doc:
https://docs.google.com/document/d/1RdUKb-deQCBiyc6Dpdf4RDBEPV-feb5WHu7uzY1YozQ/edit#heading=h.h0q5njx6l4xf
BUG=chromium:764337
TEST=Manual
CQ-DEPEND=CL:681939, CL:819250, CL:819353, CL:819412
Change-Id: I10f31d307925c6ccef3cfd0887a71b7c774139e2
Reviewed-on: https://chromium-review.googlesource.com/674935
Commit-Ready: Igor <igorcov@chromium.org>
Tested-by: Igor <igorcov@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
(cherry picked from commit d213a401f639649b3e38f6def5782ae2f1322fce)
Merged-In: I10f31d307925c6ccef3cfd0887a71b7c774139e2
diff --git a/libbrillo.gypi b/libbrillo.gypi
index 0a641ba..1788775 100644
--- a/libbrillo.gypi
+++ b/libbrillo.gypi
@@ -239,6 +239,8 @@
'sources': [
'policy/device_policy.cc',
'policy/device_policy_impl.cc',
+ 'policy/policy_util.cc',
+ 'policy/resilient_policy_util.cc',
'policy/libpolicy.cc',
],
},
@@ -374,6 +376,8 @@
'includes': ['../common-mk/common_test.gypi'],
'sources': [
'policy/tests/libpolicy_unittest.cc',
+ 'policy/tests/policy_util_unittest.cc',
+ 'policy/tests/resilient_policy_util_unittest.cc',
]
},
],
diff --git a/libpolicy.gypi b/libpolicy.gypi
index 25b251d..b3a3d49 100644
--- a/libpolicy.gypi
+++ b/libpolicy.gypi
@@ -12,6 +12,8 @@
'policy/libpolicy.h',
'policy/mock_libpolicy.h',
'policy/mock_device_policy.h',
+ 'policy/policy_util.h',
+ 'policy/resilient_policy_util.h',
],
},
],
diff --git a/policy/device_policy.h b/policy/device_policy.h
index fbebeaa..2a3cb4b 100644
--- a/policy/device_policy.h
+++ b/policy/device_policy.h
@@ -156,9 +156,6 @@
std::string* app_id_out) const = 0;
private:
- // Verifies that the policy files are owned by root and exist.
- virtual bool VerifyPolicyFiles() = 0;
-
// Verifies that the policy signature is correct.
virtual bool VerifyPolicySignature() = 0;
diff --git a/policy/device_policy_impl.cc b/policy/device_policy_impl.cc
index de29b39..ec52be6 100644
--- a/policy/device_policy_impl.cc
+++ b/policy/device_policy_impl.cc
@@ -4,9 +4,13 @@
#include "policy/device_policy_impl.h"
+#include <memory>
+
+#include <base/containers/adapters.h>
#include <base/files/file_util.h>
#include <base/logging.h>
#include <base/macros.h>
+#include <base/memory/ptr_util.h>
#include <openssl/evp.h>
#include <openssl/x509.h>
@@ -15,6 +19,8 @@
#include "bindings/chrome_device_policy.pb.h"
#include "bindings/device_management_backend.pb.h"
+#include "policy/policy_util.h"
+#include "policy/resilient_policy_util.h"
namespace policy {
@@ -64,9 +70,7 @@
int rv = EVP_VerifyInit_ex(&ctx, digest, nullptr);
if (rv == 1) {
EVP_VerifyUpdate(&ctx, signed_data.data(), signed_data.length());
- rv = EVP_VerifyFinal(&ctx,
- sig, signature.length(),
- public_key_ssl);
+ rv = EVP_VerifyFinal(&ctx, sig, signature.length(), public_key_ssl);
}
EVP_PKEY_free(public_key_ssl);
@@ -80,11 +84,7 @@
// representations. The strings must match the connection manager definitions.
std::string DecodeConnectionType(int type) {
static const char* const kConnectionTypes[] = {
- "ethernet",
- "wifi",
- "wimax",
- "bluetooth",
- "cellular",
+ "ethernet", "wifi", "wimax", "bluetooth", "cellular",
};
if (type < 0 || type >= static_cast<int>(arraysize(kConnectionTypes)))
@@ -97,40 +97,31 @@
DevicePolicyImpl::DevicePolicyImpl()
: policy_path_(kPolicyPath),
- keyfile_path_(kPublicKeyPath) {
-}
+ keyfile_path_(kPublicKeyPath),
+ verify_root_ownership_(true) {}
-DevicePolicyImpl::~DevicePolicyImpl() {
-}
+DevicePolicyImpl::~DevicePolicyImpl() {}
bool DevicePolicyImpl::LoadPolicy() {
- if (!VerifyPolicyFiles()) {
+ std::map<int, base::FilePath> sorted_policy_file_paths =
+ policy::GetSortedResilientPolicyFilePaths(policy_path_);
+ if (sorted_policy_file_paths.empty())
return false;
+
+ // Try to load the existent policy files one by one in reverse order of their
+ // index until we succeed. The default policy, if present, appears as index 0
+ // in the map and is loaded the last. This is intentional as that file is the
+ // oldest.
+ bool policy_loaded = false;
+ for (const auto& map_pair : base::Reversed(sorted_policy_file_paths)) {
+ const base::FilePath& policy_path = map_pair.second;
+ if (LoadPolicyFromFile(policy_path)) {
+ policy_loaded = true;
+ break;
+ }
}
- std::string polstr;
- if (!base::ReadFileToString(policy_path_, &polstr) || polstr.empty()) {
- LOG(ERROR) << "Could not read policy off disk";
- return false;
- }
- if (!policy_.ParseFromString(polstr) || !policy_.has_policy_data()) {
- LOG(ERROR) << "Policy on disk could not be parsed!";
- return false;
- }
- policy_data_.ParseFromString(policy_.policy_data());
- if (!policy_data_.has_policy_value()) {
- LOG(ERROR) << "Policy on disk could not be parsed!";
- return false;
- }
-
- // Make sure the signature is still valid.
- if (!VerifyPolicySignature()) {
- LOG(ERROR) << "Policy signature verification failed!";
- return false;
- }
-
- device_policy_.ParseFromString(policy_data_.policy_value());
- return true;
+ return policy_loaded;
}
bool DevicePolicyImpl::GetPolicyRefreshRate(int* rate) const {
@@ -246,8 +237,7 @@
return true;
}
-bool DevicePolicyImpl::GetReleaseChannel(
- std::string* release_channel) const {
+bool DevicePolicyImpl::GetReleaseChannel(std::string* release_channel) const {
if (!device_policy_.has_release_channel())
return false;
@@ -274,8 +264,7 @@
return true;
}
-bool DevicePolicyImpl::GetUpdateDisabled(
- bool* update_disabled) const {
+bool DevicePolicyImpl::GetUpdateDisabled(bool* update_disabled) const {
if (!device_policy_.has_auto_update_settings())
return false;
@@ -317,7 +306,7 @@
}
bool DevicePolicyImpl::GetAllowedConnectionTypesForUpdate(
- std::set<std::string>* connection_types) const {
+ std::set<std::string>* connection_types) const {
if (!device_policy_.has_auto_update_settings())
return false;
@@ -390,7 +379,7 @@
}
bool DevicePolicyImpl::GetAllowKioskAppControlChromeVersion(
- bool* allow_kiosk_app_control_chrome_version) const {
+ bool* allow_kiosk_app_control_chrome_version) const {
if (!device_policy_.has_allow_kiosk_app_control_chrome_version())
return false;
@@ -457,15 +446,19 @@
return false;
}
-bool DevicePolicyImpl::VerifyPolicyFiles() {
+bool DevicePolicyImpl::VerifyPolicyFile(const base::FilePath& policy_path) {
+ if (!verify_root_ownership_) {
+ return true;
+ }
+
// Both the policy and its signature have to exist.
- if (!base::PathExists(policy_path_) || !base::PathExists(keyfile_path_)) {
+ if (!base::PathExists(policy_path) || !base::PathExists(keyfile_path_)) {
return false;
}
// Check if the policy and signature file is owned by root.
struct stat file_stat;
- stat(policy_path_.value().c_str(), &file_stat);
+ stat(policy_path.value().c_str(), &file_stat);
if (file_stat.st_uid != 0) {
LOG(ERROR) << "Policy file is not owned by root!";
return false;
@@ -497,4 +490,46 @@
return false;
}
+bool DevicePolicyImpl::LoadPolicyFromFile(const base::FilePath& policy_path) {
+ std::string policy_data_str;
+ if (policy::LoadPolicyFromPath(policy_path, &policy_data_str, &policy_) !=
+ LoadPolicyResult::kSuccess) {
+ return false;
+ }
+ if (!policy_.has_policy_data()) {
+ LOG(ERROR) << "Policy on disk could not be parsed!";
+ return false;
+ }
+ if (!policy_data_.ParseFromString(policy_.policy_data()) ||
+ !policy_data_.has_policy_value()) {
+ LOG(ERROR) << "Policy on disk could not be parsed!";
+ return false;
+ }
+
+ bool verify_policy = true;
+ if (!install_attributes_reader_) {
+ install_attributes_reader_ = std::make_unique<InstallAttributesReader>();
+ }
+ const std::string& mode = install_attributes_reader_->GetAttribute(
+ InstallAttributesReader::kAttrMode);
+ if (mode == InstallAttributesReader::kDeviceModeEnterpriseAD) {
+ verify_policy = false;
+ }
+ if (verify_policy && !VerifyPolicyFile(policy_path)) {
+ return false;
+ }
+
+ // Make sure the signature is still valid.
+ if (verify_policy && !VerifyPolicySignature()) {
+ LOG(ERROR) << "Policy signature verification failed!";
+ return false;
+ }
+ if (!device_policy_.ParseFromString(policy_data_.policy_value())) {
+ LOG(ERROR) << "Policy on disk could not be parsed!";
+ return false;
+ }
+
+ return true;
+}
+
} // namespace policy
diff --git a/policy/device_policy_impl.h b/policy/device_policy_impl.h
index da7626a..b178e28 100644
--- a/policy/device_policy_impl.h
+++ b/policy/device_policy_impl.h
@@ -63,24 +63,52 @@
bool* allow_kiosk_app_control_chrome_version) const override;
bool GetUsbDetachableWhitelist(
std::vector<UsbDeviceId>* usb_whitelist) const override;
- bool GetAutoLaunchedKioskAppId(
- std::string* app_id_out) const override;
+ bool GetAutoLaunchedKioskAppId(std::string* app_id_out) const override;
- protected:
- // Verifies that the policy files are owned by root and exist.
- bool VerifyPolicyFiles() override;
-
- base::FilePath policy_path_;
- base::FilePath keyfile_path_;
+ // Methods that can be used only for testing.
+ void set_policy_data_for_testing(
+ const enterprise_management::PolicyData& policy_data) {
+ policy_data_ = policy_data;
+ }
+ void set_verify_root_ownership_for_testing(bool verify_root_ownership) {
+ verify_root_ownership_ = verify_root_ownership;
+ }
+ void set_install_attributes_for_testing(
+ std::unique_ptr<InstallAttributesReader> install_attributes_reader) {
+ install_attributes_reader_ = std::move(install_attributes_reader);
+ }
+ void set_policy_path_for_testing(const base::FilePath& policy_path) {
+ policy_path_ = policy_path;
+ }
+ void set_key_file_path_for_testing(const base::FilePath& keyfile_path) {
+ keyfile_path_ = keyfile_path;
+ }
private:
+ // Verifies that both the policy file and the signature file exist and are
+ // owned by the root. Does nothing when |verify_root_ownership_| is set to
+ // false.
+ bool VerifyPolicyFile(const base::FilePath& policy_path);
+
// Verifies that the policy signature is correct.
bool VerifyPolicySignature() override;
+ // Loads the signed policy off of disk from |policy_path| into |policy_|.
+ // Returns true if the |policy_path| is present on disk and loading it is
+ // successful.
+ bool LoadPolicyFromFile(const base::FilePath& policy_path);
+
+ base::FilePath policy_path_;
+ base::FilePath keyfile_path_;
+ std::unique_ptr<InstallAttributesReader> install_attributes_reader_;
enterprise_management::PolicyFetchResponse policy_;
enterprise_management::PolicyData policy_data_;
enterprise_management::ChromeDeviceSettingsProto device_policy_;
+ // If true, verify that policy files are owned by root. True in production
+ // but can be set to false by tests.
+ bool verify_root_ownership_;
+
DISALLOW_COPY_AND_ASSIGN(DevicePolicyImpl);
};
} // namespace policy
diff --git a/policy/libpolicy.cc b/policy/libpolicy.cc
index c075c84..99fa46d 100644
--- a/policy/libpolicy.cc
+++ b/policy/libpolicy.cc
@@ -21,13 +21,11 @@
#endif
}
-PolicyProvider::PolicyProvider(DevicePolicy* device_policy)
- : device_policy_(device_policy),
- device_policy_is_loaded_(true) {
-}
+PolicyProvider::PolicyProvider(std::unique_ptr<DevicePolicy> device_policy)
+ : device_policy_(std::move(device_policy)),
+ device_policy_is_loaded_(true) {}
-PolicyProvider::~PolicyProvider() {
-}
+PolicyProvider::~PolicyProvider() {}
bool PolicyProvider::Reload() {
if (!device_policy_)
diff --git a/policy/libpolicy.h b/policy/libpolicy.h
index eeca189..1a411cf 100644
--- a/policy/libpolicy.h
+++ b/policy/libpolicy.h
@@ -26,7 +26,7 @@
virtual ~PolicyProvider();
// Constructor for tests only!
- explicit PolicyProvider(DevicePolicy* device_policy);
+ explicit PolicyProvider(std::unique_ptr<DevicePolicy> device_policy);
// This function will ensure the freshness of the contents that the getters
// are delivering. Normally contents are cached to prevent unnecessary load.
diff --git a/policy/policy_util.cc b/policy/policy_util.cc
new file mode 100644
index 0000000..8c17fc2
--- /dev/null
+++ b/policy/policy_util.cc
@@ -0,0 +1,41 @@
+// Copyright 2017 The Chromium OS 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 "policy/policy_util.h"
+
+#include <base/files/file_util.h>
+#include <base/logging.h>
+
+namespace policy {
+
+LoadPolicyResult LoadPolicyFromPath(
+ const base::FilePath& policy_path,
+ std::string* policy_data_str_out,
+ enterprise_management::PolicyFetchResponse* policy_out) {
+ DCHECK(policy_data_str_out);
+ DCHECK(policy_out);
+ if (!base::PathExists(policy_path)) {
+ return LoadPolicyResult::kFileNotFound;
+ }
+
+ if (!base::ReadFileToString(policy_path, policy_data_str_out)) {
+ PLOG(ERROR) << "Could not read policy off disk at " << policy_path.value();
+ return LoadPolicyResult::kFailedToReadFile;
+ }
+
+ if (policy_data_str_out->empty()) {
+ LOG(ERROR) << "Empty policy file at " << policy_path.value();
+ return LoadPolicyResult::kEmptyFile;
+ }
+
+ if (!policy_out->ParseFromString(*policy_data_str_out)) {
+ LOG(ERROR) << "Policy on disk could not be parsed, file: "
+ << policy_path.value();
+ return LoadPolicyResult::kInvalidPolicyData;
+ }
+
+ return LoadPolicyResult::kSuccess;
+}
+
+} // namespace policy
diff --git a/policy/policy_util.h b/policy/policy_util.h
new file mode 100644
index 0000000..e1038fd
--- /dev/null
+++ b/policy/policy_util.h
@@ -0,0 +1,38 @@
+// Copyright 2017 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef LIBBRILLO_POLICY_POLICY_UTIL_H_
+#define LIBBRILLO_POLICY_POLICY_UTIL_H_
+
+#include <string>
+
+#include <base/files/file_path.h>
+#include <brillo/brillo_export.h>
+
+#include "bindings/device_management_backend.pb.h"
+
+namespace policy {
+
+// The detailed information of the result from loading the policy data.
+enum class BRILLO_EXPORT LoadPolicyResult {
+ kSuccess = 0,
+ kFileNotFound = 1,
+ kFailedToReadFile = 2,
+ kEmptyFile = 3,
+ kInvalidPolicyData = 4
+};
+
+// Reads and parses the policy data from |policy_path|. Returns the details
+// in LoadPolicyResult. In case response is |kSuccess|, |policy_data_str_out|
+// contains the raw data from the file, while |policy_out| contains the parsed
+// policy data. Otherwise the contents of |policy_data_str_out| and |policy_out|
+// is undefined.
+BRILLO_EXPORT LoadPolicyResult LoadPolicyFromPath(
+ const base::FilePath& policy_path,
+ std::string* policy_data_str_out,
+ enterprise_management::PolicyFetchResponse* policy_out);
+
+} // namespace policy
+
+#endif // LIBBRILLO_POLICY_POLICY_UTIL_H_
diff --git a/policy/resilient_policy_util.cc b/policy/resilient_policy_util.cc
new file mode 100644
index 0000000..4a7ec42
--- /dev/null
+++ b/policy/resilient_policy_util.cc
@@ -0,0 +1,71 @@
+// Copyright 2017 The Chromium OS 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 "policy/resilient_policy_util.h"
+
+#include <algorithm>
+
+#include <base/files/file_enumerator.h>
+#include <base/files/file_util.h>
+#include <base/strings/string_number_conversions.h>
+#include <base/strings/string_util.h>
+
+namespace policy {
+
+std::map<int, base::FilePath> GetSortedResilientPolicyFilePaths(
+ const base::FilePath& default_policy_path) {
+ std::map<int, base::FilePath> sorted_policy_file_paths;
+ base::FilePath directory = default_policy_path.DirName();
+ if (!base::PathExists(directory))
+ return sorted_policy_file_paths;
+
+ // Iterate the list of files in the folder, identifying the policy files based
+ // on the name. Store in the map the absolute paths.
+ const std::string default_policy_file_name =
+ default_policy_path.BaseName().value();
+ base::FileEnumerator file_iter(directory, false, base::FileEnumerator::FILES);
+ for (base::FilePath child_file = file_iter.Next(); !child_file.empty();
+ child_file = file_iter.Next()) {
+ int index;
+ if (ParseResilientPolicyFilePath(child_file, default_policy_path, &index)) {
+ sorted_policy_file_paths[index] = child_file;
+ }
+ }
+
+ return sorted_policy_file_paths;
+}
+
+bool ParseResilientPolicyFilePath(const base::FilePath& policy_path,
+ const base::FilePath& default_policy_path,
+ int* index_out) {
+ if (!base::StartsWith(policy_path.value(), default_policy_path.value(),
+ base::CompareCase::SENSITIVE)) {
+ return false;
+ }
+
+ if (policy_path == default_policy_path) {
+ *index_out = 0;
+ return true;
+ }
+
+ const std::string extension =
+ policy_path.value().substr(default_policy_path.value().size());
+ if (extension.empty() || extension[0] != '.' ||
+ !base::StringToInt(extension.substr(1), index_out) || *index_out <= 0) {
+ return false;
+ }
+
+ return true;
+}
+
+base::FilePath GetResilientPolicyFilePathForIndex(
+ const base::FilePath& default_policy_path,
+ int index) {
+ if (index == 0)
+ return default_policy_path;
+ return base::FilePath(default_policy_path.value() + "." +
+ std::to_string(index));
+}
+
+} // namespace policy
diff --git a/policy/resilient_policy_util.h b/policy/resilient_policy_util.h
new file mode 100644
index 0000000..9df4f54
--- /dev/null
+++ b/policy/resilient_policy_util.h
@@ -0,0 +1,40 @@
+// Copyright 2017 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef LIBBRILLO_POLICY_RESILIENT_POLICY_UTIL_H_
+#define LIBBRILLO_POLICY_RESILIENT_POLICY_UTIL_H_
+
+#include <map>
+#include <string>
+
+#include <base/files/file_path.h>
+#include <brillo/brillo_export.h>
+
+namespace policy {
+
+// Returns a map from policy file index to absolute path. The default policy
+// file is included at index 0 if present (despite not having an index in its
+// filename).
+BRILLO_EXPORT std::map<int, base::FilePath> GetSortedResilientPolicyFilePaths(
+ const base::FilePath& default_policy_path);
+
+// Returns the path to policy file corresponding to |index| value, based on
+// the path of the default policy given by |default_policy_path|. Doesn't check
+// the existence of the file on disk.
+BRILLO_EXPORT base::FilePath GetResilientPolicyFilePathForIndex(
+ const base::FilePath& default_policy_path,
+ int index);
+
+// Returns whether the |policy_path| file is a resilient file based on the name
+// of the file, assuming the |default_policy_path| is the path of the default
+// policy file. If successful, the |index_out| contains the index of the file as
+// deducted from the name. No parsing of file contents is done here.
+BRILLO_EXPORT bool ParseResilientPolicyFilePath(
+ const base::FilePath& policy_path,
+ const base::FilePath& default_policy_path,
+ int* index_out);
+
+} // namespace policy
+
+#endif // LIBBRILLO_POLICY_RESILIENT_POLICY_UTIL_H_
diff --git a/policy/tests/libpolicy_unittest.cc b/policy/tests/libpolicy_unittest.cc
index 4b80e7a..bd2b49a 100644
--- a/policy/tests/libpolicy_unittest.cc
+++ b/policy/tests/libpolicy_unittest.cc
@@ -15,43 +15,35 @@
namespace policy {
-static const char kPolicyFileAllSet[] =
- "policy/tests/whitelist/policy_all";
-static const char kPolicyFileNoneSet[] =
- "policy/tests/whitelist/policy_none";
+static const char kPolicyFileAllSet[] = "policy/tests/whitelist/policy_all";
+static const char kPolicyFileNoneSet[] = "policy/tests/whitelist/policy_none";
static const char kKeyFile[] = "policy/tests/whitelist/owner.key";
-// This class mocks only the minimally needed functionionality to run tests
-// that would otherwise fail because of hard restrictions like root file
-// ownership. Otherwise, it preserves all the functionallity of the original
-// class.
-class MockDevicePolicyImpl : public DevicePolicyImpl {
- public:
- MockDevicePolicyImpl(const base::FilePath& policy_path,
- const base::FilePath& keyfile_path,
- bool verify_files)
- : verify_files_(verify_files) {
- policy_path_ = policy_path;
- keyfile_path_ = keyfile_path;
- }
+// Creates the DevicePolicyImpl with given parameters for test.
+std::unique_ptr<DevicePolicyImpl> CreateDevicePolicyImpl(
+ std::unique_ptr<InstallAttributesReader> install_attributes_reader,
+ const base::FilePath& policy_path,
+ const base::FilePath& keyfile_path,
+ bool verify_files) {
+ std::unique_ptr<DevicePolicyImpl> device_policy(new DevicePolicyImpl());
+ device_policy->set_install_attributes_for_testing(
+ std::move(install_attributes_reader));
+ device_policy->set_policy_path_for_testing(policy_path);
+ device_policy->set_key_file_path_for_testing(keyfile_path);
+ device_policy->set_verify_root_ownership_for_testing(verify_files);
- private:
- // We don't care if files are owned by root for most tests.
- virtual bool VerifyPolicyFiles() {
- return !verify_files_ || DevicePolicyImpl::VerifyPolicyFiles();
- }
-
- bool verify_files_;
-};
+ return device_policy;
+}
// Test that a policy file can be verified and parsed correctly. The file
// contains all possible fields, so reading should succeed for all.
TEST(PolicyTest, DevicePolicyAllSetTest) {
base::FilePath policy_file(kPolicyFileAllSet);
base::FilePath key_file(kKeyFile);
- MockDevicePolicyImpl* device_policy =
- new MockDevicePolicyImpl(policy_file, key_file, false);
- PolicyProvider provider(device_policy);
+ PolicyProvider provider(
+ CreateDevicePolicyImpl(std::make_unique<MockInstallAttributesReader>(
+ cryptohome::SerializedInstallAttributes()),
+ policy_file, key_file, false));
provider.Reload();
// Ensure we successfully loaded the device policy file.
@@ -174,9 +166,11 @@
TEST(PolicyTest, DevicePolicyNoneSetTest) {
base::FilePath policy_file(kPolicyFileNoneSet);
base::FilePath key_file(kKeyFile);
- MockDevicePolicyImpl* device_policy =
- new MockDevicePolicyImpl(policy_file, key_file, false);
- PolicyProvider provider(device_policy);
+
+ PolicyProvider provider(
+ CreateDevicePolicyImpl(std::make_unique<MockInstallAttributesReader>(
+ cryptohome::SerializedInstallAttributes()),
+ policy_file, key_file, false));
provider.Reload();
// Ensure we successfully loaded the device policy file.
@@ -220,9 +214,13 @@
LOG(INFO) << "Errors expected.";
// Try loading non-existing protobuf should fail.
base::FilePath non_existing("this_file_is_doof");
- MockDevicePolicyImpl* device_policy =
- new MockDevicePolicyImpl(non_existing, non_existing, true);
- PolicyProvider provider(device_policy);
+ PolicyProvider provider(
+ CreateDevicePolicyImpl(std::make_unique<MockInstallAttributesReader>(
+ cryptohome::SerializedInstallAttributes()),
+ non_existing,
+ non_existing,
+ true));
+
// Even after reload the policy should still be not loaded.
ASSERT_FALSE(provider.Reload());
ASSERT_FALSE(provider.device_policy_is_loaded());
diff --git a/policy/tests/policy_util_unittest.cc b/policy/tests/policy_util_unittest.cc
new file mode 100644
index 0000000..74032d4
--- /dev/null
+++ b/policy/tests/policy_util_unittest.cc
@@ -0,0 +1,55 @@
+// Copyright 2017 The Chromium OS 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 <gtest/gtest.h>
+
+#include <base/files/file_util.h>
+#include <base/files/scoped_temp_dir.h>
+
+#include "policy/policy_util.h"
+
+namespace em = enterprise_management;
+
+namespace policy {
+
+// Test LoadPolicyFromPath returns correct values and has policy data when
+// successful.
+TEST(DevicePolicyUtilTest, LoadPolicyFromPath) {
+ // Create the temporary directory.
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+ base::FilePath invalid_policy_data_path(temp_dir.path().Append("policy"));
+ base::FilePath inexistent_file(temp_dir.path().Append("policy.1"));
+ base::FilePath good_policy_data_path(temp_dir.path().Append("policy.2"));
+
+ // Create the file with invalid data.
+ std::string data = "invalid data";
+ ASSERT_TRUE(
+ base::WriteFile(invalid_policy_data_path, data.data(), data.size()));
+
+ // Create the file with good policy data.
+ em::PolicyData policy_data;
+ policy_data.set_username("user@example.com");
+ policy_data.set_management_mode(em::PolicyData::LOCAL_OWNER);
+ policy_data.set_request_token("codepath-must-ignore-dmtoken");
+ std::string policy_blob;
+ policy_data.SerializeToString(&policy_blob);
+ ASSERT_TRUE(base::WriteFile(good_policy_data_path, policy_blob.data(),
+ policy_blob.size()));
+
+ std::string policy_data_str;
+ enterprise_management::PolicyFetchResponse policy;
+ EXPECT_EQ(
+ LoadPolicyResult::kInvalidPolicyData,
+ LoadPolicyFromPath(invalid_policy_data_path, &policy_data_str, &policy));
+ EXPECT_EQ(LoadPolicyResult::kFileNotFound,
+ LoadPolicyFromPath(inexistent_file, &policy_data_str, &policy));
+ EXPECT_EQ(
+ LoadPolicyResult::kSuccess,
+ LoadPolicyFromPath(good_policy_data_path, &policy_data_str, &policy));
+ ASSERT_TRUE(policy.has_policy_data());
+}
+
+} // namespace policy
diff --git a/policy/tests/resilient_policy_util_unittest.cc b/policy/tests/resilient_policy_util_unittest.cc
new file mode 100644
index 0000000..e45ab34
--- /dev/null
+++ b/policy/tests/resilient_policy_util_unittest.cc
@@ -0,0 +1,58 @@
+// Copyright 2017 The Chromium OS 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 <gtest/gtest.h>
+
+#include <map>
+#include <string>
+
+#include <base/files/file_path.h>
+#include <base/files/file_util.h>
+#include <base/files/scoped_temp_dir.h>
+
+#include "policy/resilient_policy_util.h"
+
+namespace {
+
+const char kDefaultResilientPolicyFilePath[] = "policy";
+
+void CreateFile(const base::FilePath& file_path) {
+ base::File file(file_path, base::File::FLAG_CREATE | base::File::FLAG_WRITE);
+}
+
+} // namespace
+
+namespace policy {
+
+// Test that the policy files from the folder are identified correctly.
+TEST(DevicePolicyUtilTest, GetSortedResilientPolicyFilePaths) {
+ // Create the temporary directory.
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+ base::FilePath file0(temp_dir.path().Append("policy"));
+ base::FilePath file1(temp_dir.path().Append("policy.12"));
+ base::FilePath file2(temp_dir.path().Append("policy.2"));
+ base::FilePath file3(temp_dir.path().Append("policy.30"));
+ base::FilePath invalid(temp_dir.path().Append("policy_4"));
+
+ CreateFile(file0);
+ CreateFile(file1);
+ CreateFile(file2);
+ CreateFile(file3);
+
+ const base::FilePath test_file_path(
+ temp_dir.path().Append(kDefaultResilientPolicyFilePath));
+ std::map<int, base::FilePath> sorted_file_paths =
+ GetSortedResilientPolicyFilePaths(test_file_path);
+
+ EXPECT_EQ(4, sorted_file_paths.size());
+ EXPECT_EQ(file0.value(), sorted_file_paths[0].value());
+ EXPECT_EQ(file1.value(), sorted_file_paths[12].value());
+ EXPECT_EQ(file2.value(), sorted_file_paths[2].value());
+ EXPECT_EQ(file3.value(), sorted_file_paths[30].value());
+ EXPECT_EQ("", sorted_file_paths[4].value());
+}
+
+} // namespace policy
