diff options
author | Igor <igorcov@chromium.org> | 2018-01-12 01:28:52 +0000 |
---|---|---|
committer | android-build-merger <android-build-merger@google.com> | 2018-01-12 01:28:52 +0000 |
commit | 1310886f7e2537dc5f05c8313450bbb28b44c465 (patch) | |
tree | 952188b7d5d0bc6cf1601731a7aadd1f7cf4345e | |
parent | 957cd3c9291476b163ef5428e746cf39ac6ad379 (diff) | |
parent | 0cd09dcb4449e0afcaf74cbbd5a993846e906e51 (diff) | |
download | platform_external_libbrillo-1310886f7e2537dc5f05c8313450bbb28b44c465.tar.gz platform_external_libbrillo-1310886f7e2537dc5f05c8313450bbb28b44c465.tar.bz2 platform_external_libbrillo-1310886f7e2537dc5f05c8313450bbb28b44c465.zip |
libbrillo: policy: Increase resilience for policy read
am: 0cd09dcb44
Change-Id: Ia6d0ba65fec2bd4e86317bbcb4ed5f349ad593d5
-rw-r--r-- | libbrillo.gypi | 4 | ||||
-rw-r--r-- | libpolicy.gypi | 2 | ||||
-rw-r--r-- | policy/device_policy.h | 3 | ||||
-rw-r--r-- | policy/device_policy_impl.cc | 127 | ||||
-rw-r--r-- | policy/device_policy_impl.h | 44 | ||||
-rw-r--r-- | policy/libpolicy.cc | 10 | ||||
-rw-r--r-- | policy/libpolicy.h | 2 | ||||
-rw-r--r-- | policy/policy_util.cc | 41 | ||||
-rw-r--r-- | policy/policy_util.h | 38 | ||||
-rw-r--r-- | policy/resilient_policy_util.cc | 71 | ||||
-rw-r--r-- | policy/resilient_policy_util.h | 40 | ||||
-rw-r--r-- | policy/tests/libpolicy_unittest.cc | 68 | ||||
-rw-r--r-- | policy/tests/policy_util_unittest.cc | 55 | ||||
-rw-r--r-- | policy/tests/resilient_policy_util_unittest.cc | 58 |
14 files changed, 464 insertions, 99 deletions
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 @@ class DevicePolicy { 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 @@ bool VerifySignature(const std::string& signed_data, 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 @@ bool VerifySignature(const std::string& signed_data, // 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 @@ std::string DecodeConnectionType(int type) { DevicePolicyImpl::DevicePolicyImpl() : policy_path_(kPolicyPath), - keyfile_path_(kPublicKeyPath) { -} + keyfile_path_(kPublicKeyPath), + verify_root_ownership_(true) {} -DevicePolicyImpl::~DevicePolicyImpl() { -} +DevicePolicyImpl::~DevicePolicyImpl() {} bool DevicePolicyImpl::LoadPolicy() { - if (!VerifyPolicyFiles()) { - return false; - } - - 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; + 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; + } } - device_policy_.ParseFromString(policy_data_.policy_value()); - return true; + return policy_loaded; } bool DevicePolicyImpl::GetPolicyRefreshRate(int* rate) const { @@ -246,8 +237,7 @@ bool DevicePolicyImpl::GetEphemeralUsersEnabled( 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 @@ bool DevicePolicyImpl::GetReleaseChannelDelegated( 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::GetScatterFactorInSeconds( } 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::GetAuP2PEnabled(bool* au_p2p_enabled) const { } 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 @@ bool DevicePolicyImpl::GetAutoLaunchedKioskAppId( 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 @@ bool DevicePolicyImpl::VerifyPolicySignature() { 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 @@ class DevicePolicyImpl : public DevicePolicy { 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 @@ PolicyProvider::PolicyProvider() #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 @@ class PolicyProvider { 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; - } - - 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_; -}; +// 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); + + 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, DevicePolicyAllSetTest) { 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 @@ TEST(PolicyTest, DevicePolicyFailure) { 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 |