Commit b5135d41 authored by Nikita Podguzov's avatar Nikita Podguzov Committed by Commit Bot

Add cache size check before storing object in ResourceCache.

Bug: 807068
Change-Id: I1e55bdea38b02d5fdf1d7faa9a60390af5fac7d7
Reviewed-on: https://chromium-review.googlesource.com/c/1290983Reviewed-by: default avatarSean Kau <skau@chromium.org>
Reviewed-by: default avatarMaksim Ivanov <emaxx@chromium.org>
Reviewed-by: default avatarSergey Poromov <poromov@chromium.org>
Commit-Queue: Nikita Podguzov <nikitapodguzov@google.com>
Cr-Commit-Position: refs/heads/master@{#605646}
parent d3bf809a
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/optional.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/test/test_simple_task_runner.h" #include "base/test/test_simple_task_runner.h"
...@@ -110,8 +111,9 @@ CloudExternalDataManagerBaseTest::CloudExternalDataManagerBaseTest() { ...@@ -110,8 +111,9 @@ CloudExternalDataManagerBaseTest::CloudExternalDataManagerBaseTest() {
void CloudExternalDataManagerBaseTest::SetUp() { void CloudExternalDataManagerBaseTest::SetUp() {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
resource_cache_.reset( resource_cache_.reset(new ResourceCache(temp_dir_.GetPath(),
new ResourceCache(temp_dir_.GetPath(), message_loop_.task_runner())); message_loop_.task_runner(),
/* max_cache_size */ base::nullopt));
SetUpExternalDataManager(); SetUpExternalDataManager();
// Set |kStringPolicy| to a string value. // Set |kStringPolicy| to a string value.
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h"
#include "base/test/test_simple_task_runner.h" #include "base/test/test_simple_task_runner.h"
#include "components/policy/core/common/cloud/resource_cache.h" #include "components/policy/core/common/cloud/resource_cache.h"
#include "crypto/sha2.h" #include "crypto/sha2.h"
...@@ -57,7 +58,8 @@ CouldExternalDataStoreTest::CouldExternalDataStoreTest() ...@@ -57,7 +58,8 @@ CouldExternalDataStoreTest::CouldExternalDataStoreTest()
void CouldExternalDataStoreTest::SetUp() { void CouldExternalDataStoreTest::SetUp() {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
resource_cache_.reset(new ResourceCache(temp_dir_.GetPath(), task_runner_)); resource_cache_.reset(new ResourceCache(temp_dir_.GetPath(), task_runner_,
/* max_cache_size */ base::nullopt));
} }
TEST_F(CouldExternalDataStoreTest, StoreAndLoad) { TEST_F(CouldExternalDataStoreTest, StoreAndLoad) {
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/optional.h"
#include "base/path_service.h" #include "base/path_service.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/null_task_runner.h" #include "base/test/null_task_runner.h"
...@@ -419,7 +420,8 @@ class SigninExtensionsDeviceCloudPolicyBrowserTest ...@@ -419,7 +420,8 @@ class SigninExtensionsDeviceCloudPolicyBrowserTest
void TearDownInProcessBrowserTestFixture() override { void TearDownInProcessBrowserTestFixture() override {
// Check that the component policy cache was not cleared during browser // Check that the component policy cache was not cleared during browser
// teardown. // teardown.
ResourceCache cache(component_policy_cache_dir_, new base::NullTaskRunner); ResourceCache cache(component_policy_cache_dir_, new base::NullTaskRunner,
/* max_cache_size */ base::nullopt);
std::string stub; std::string stub;
EXPECT_TRUE(cache.Load(kPolicyProtoCacheKey, kTestExtensionId, &stub)); EXPECT_TRUE(cache.Load(kPolicyProtoCacheKey, kTestExtensionId, &stub));
EXPECT_TRUE(cache.Load(kPolicyDataCacheKey, kTestExtensionId, &stub)); EXPECT_TRUE(cache.Load(kPolicyDataCacheKey, kTestExtensionId, &stub));
...@@ -534,7 +536,8 @@ class PreinstalledSigninExtensionsDeviceCloudPolicyBrowserTest ...@@ -534,7 +536,8 @@ class PreinstalledSigninExtensionsDeviceCloudPolicyBrowserTest
EXPECT_TRUE(base::PathService::Get( EXPECT_TRUE(base::PathService::Get(
chromeos::DIR_SIGNIN_PROFILE_COMPONENT_POLICY, &cache_dir)); chromeos::DIR_SIGNIN_PROFILE_COMPONENT_POLICY, &cache_dir));
ResourceCache cache(cache_dir, new base::NullTaskRunner); ResourceCache cache(cache_dir, new base::NullTaskRunner,
/* max_cache_size */ base::nullopt);
EXPECT_TRUE(cache.Store(kPolicyProtoCacheKey, kTestExtensionId, EXPECT_TRUE(cache.Store(kPolicyProtoCacheKey, kTestExtensionId,
BuildTestComponentPolicy().SerializeAsString())); BuildTestComponentPolicy().SerializeAsString()));
EXPECT_TRUE( EXPECT_TRUE(
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/location.h" #include "base/location.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/optional.h"
#include "base/path_service.h" #include "base/path_service.h"
#include "base/sequenced_task_runner.h" #include "base/sequenced_task_runner.h"
#include "chromeos/chromeos_paths.h" #include "chromeos/chromeos_paths.h"
...@@ -27,7 +28,8 @@ DeviceLocalAccountExternalDataService::DeviceLocalAccountExternalDataService( ...@@ -27,7 +28,8 @@ DeviceLocalAccountExternalDataService::DeviceLocalAccountExternalDataService(
base::FilePath cache_dir; base::FilePath cache_dir;
CHECK(base::PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA, CHECK(base::PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA,
&cache_dir)); &cache_dir));
resource_cache_.reset(new ResourceCache(cache_dir, backend_task_runner_)); resource_cache_.reset(new ResourceCache(cache_dir, backend_task_runner_,
/* max_cache_size */ base::nullopt));
parent_->AddObserver(this); parent_->AddObserver(this);
} }
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/files/file_enumerator.h" #include "base/files/file_enumerator.h"
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/optional.h"
#include "base/path_service.h" #include "base/path_service.h"
#include "base/sequenced_task_runner.h" #include "base/sequenced_task_runner.h"
#include "base/stl_util.h" #include "base/stl_util.h"
...@@ -250,7 +251,8 @@ void DeviceLocalAccountPolicyBroker::CreateComponentCloudPolicyService( ...@@ -250,7 +251,8 @@ void DeviceLocalAccountPolicyBroker::CreateComponentCloudPolicyService(
} }
std::unique_ptr<ResourceCache> resource_cache(new ResourceCache( std::unique_ptr<ResourceCache> resource_cache(new ResourceCache(
component_policy_cache_path_, resource_cache_task_runner_)); component_policy_cache_path_, resource_cache_task_runner_,
/* max_cache_size */ base::nullopt));
component_policy_service_.reset(new ComponentCloudPolicyService( component_policy_service_.reset(new ComponentCloudPolicyService(
dm_protocol::kChromeExtensionPolicyType, this, &schema_registry_, core(), dm_protocol::kChromeExtensionPolicyType, this, &schema_registry_, core(),
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <memory> #include <memory>
#include "base/location.h" #include "base/location.h"
#include "base/optional.h"
#include "base/sequenced_task_runner.h" #include "base/sequenced_task_runner.h"
#include "chrome/browser/chromeos/policy/cloud_external_data_store.h" #include "chrome/browser/chromeos/policy/cloud_external_data_store.h"
#include "components/policy/core/common/cloud/cloud_policy_store.h" #include "components/policy/core/common/cloud/cloud_policy_store.h"
...@@ -26,7 +27,9 @@ UserCloudExternalDataManager::UserCloudExternalDataManager( ...@@ -26,7 +27,9 @@ UserCloudExternalDataManager::UserCloudExternalDataManager(
const base::FilePath& cache_path, const base::FilePath& cache_path,
CloudPolicyStore* policy_store) CloudPolicyStore* policy_store)
: CloudExternalDataManagerBase(get_policy_details, backend_task_runner), : CloudExternalDataManagerBase(get_policy_details, backend_task_runner),
resource_cache_(new ResourceCache(cache_path, backend_task_runner)) { resource_cache_(new ResourceCache(cache_path,
backend_task_runner,
/* max_cache_size */ base::nullopt)) {
SetPolicyStore(policy_store); SetPolicyStore(policy_store);
SetExternalDataStore(std::make_unique<CloudExternalDataStore>( SetExternalDataStore(std::make_unique<CloudExternalDataStore>(
kCacheKey, backend_task_runner, resource_cache_)); kCacheKey, backend_task_runner, resource_cache_));
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/optional.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "base/task/task_traits.h" #include "base/task/task_traits.h"
#include "build/build_config.h" #include "build/build_config.h"
...@@ -145,8 +146,8 @@ void CloudPolicyManager::CreateComponentCloudPolicyService( ...@@ -145,8 +146,8 @@ void CloudPolicyManager::CreateComponentCloudPolicyService(
// on the same task runner. // on the same task runner.
const auto task_runner = const auto task_runner =
base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}); base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()});
std::unique_ptr<ResourceCache> resource_cache( std::unique_ptr<ResourceCache> resource_cache(new ResourceCache(
new ResourceCache(policy_cache_path, task_runner)); policy_cache_path, task_runner, /* max_cache_size */ base::nullopt));
component_policy_service_.reset(new ComponentCloudPolicyService( component_policy_service_.reset(new ComponentCloudPolicyService(
policy_type, this, schema_registry, core(), client, policy_type, this, schema_registry, core(), client,
std::move(resource_cache), task_runner)); std::move(resource_cache), task_runner));
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/optional.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/stl_util.h" #include "base/stl_util.h"
...@@ -121,8 +122,9 @@ class ComponentCloudPolicyServiceTest : public testing::Test { ...@@ -121,8 +122,9 @@ class ComponentCloudPolicyServiceTest : public testing::Test {
void SetUp() override { void SetUp() override {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
owned_cache_.reset( owned_cache_.reset(new ResourceCache(temp_dir_.GetPath(),
new ResourceCache(temp_dir_.GetPath(), loop_.task_runner())); loop_.task_runner(),
/* max_cache_size */ base::nullopt));
cache_ = owned_cache_.get(); cache_ = owned_cache_.get();
} }
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/optional.h"
#include "base/test/test_simple_task_runner.h" #include "base/test/test_simple_task_runner.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "components/account_id/account_id.h" #include "components/account_id/account_id.h"
...@@ -93,9 +94,9 @@ class ComponentCloudPolicyStoreTest : public testing::Test { ...@@ -93,9 +94,9 @@ class ComponentCloudPolicyStoreTest : public testing::Test {
void SetUp() override { void SetUp() override {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
cache_.reset( cache_.reset(new ResourceCache(
new ResourceCache(temp_dir_.GetPath(), temp_dir_.GetPath(), base::MakeRefCounted<base::TestSimpleTaskRunner>(),
base::MakeRefCounted<base::TestSimpleTaskRunner>())); /* max_cache_size */ base::nullopt));
store_ = CreateStore(); store_ = CreateStore();
store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(), store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(),
PolicyBuilder::kFakeToken, PolicyBuilder::kFakeToken,
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/optional.h"
#include "base/sequenced_task_runner.h" #include "base/sequenced_task_runner.h"
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "base/time/time.h" #include "base/time/time.h"
...@@ -117,7 +118,8 @@ ComponentCloudPolicyUpdaterTest::ComponentCloudPolicyUpdaterTest() ...@@ -117,7 +118,8 @@ ComponentCloudPolicyUpdaterTest::ComponentCloudPolicyUpdaterTest()
void ComponentCloudPolicyUpdaterTest::SetUp() { void ComponentCloudPolicyUpdaterTest::SetUp() {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
cache_ = std::make_unique<ResourceCache>(temp_dir_.GetPath(), cache_ = std::make_unique<ResourceCache>(temp_dir_.GetPath(),
task_env_.GetMainThreadTaskRunner()); task_env_.GetMainThreadTaskRunner(),
/* max_cache_size */ base::nullopt);
store_ = std::make_unique<ComponentCloudPolicyStore>( store_ = std::make_unique<ComponentCloudPolicyStore>(
&store_delegate_, cache_.get(), dm_protocol::kChromeExtensionPolicyType); &store_delegate_, cache_.get(), dm_protocol::kChromeExtensionPolicyType);
store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(), store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(),
......
...@@ -5,8 +5,10 @@ ...@@ -5,8 +5,10 @@
#include "components/policy/core/common/cloud/resource_cache.h" #include "components/policy/core/common/cloud/resource_cache.h"
#include "base/base64url.h" #include "base/base64url.h"
#include "base/bind.h"
#include "base/callback.h" #include "base/callback.h"
#include "base/files/file_enumerator.h" #include "base/files/file_enumerator.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/numerics/safe_conversions.h" #include "base/numerics/safe_conversions.h"
...@@ -41,9 +43,18 @@ bool Base64UrlEncode(const std::set<std::string>& input, ...@@ -41,9 +43,18 @@ bool Base64UrlEncode(const std::set<std::string>& input,
ResourceCache::ResourceCache( ResourceCache::ResourceCache(
const base::FilePath& cache_dir, const base::FilePath& cache_dir,
scoped_refptr<base::SequencedTaskRunner> task_runner) scoped_refptr<base::SequencedTaskRunner> task_runner,
base::Optional<int64_t> max_cache_size)
: cache_dir_(cache_dir), : cache_dir_(cache_dir),
task_runner_(task_runner) { task_runner_(task_runner),
max_cache_size_(max_cache_size) {
// Safe to post this without a WeakPtr because this class must be destructed
// on the same thread.
if (max_cache_size_.has_value()) {
task_runner_->PostTask(FROM_HERE,
base::BindOnce(&ResourceCache::InitCurrentCacheSize,
base::Unretained(this)));
}
} }
ResourceCache::~ResourceCache() { ResourceCache::~ResourceCache() {
...@@ -55,19 +66,18 @@ bool ResourceCache::Store(const std::string& key, ...@@ -55,19 +66,18 @@ bool ResourceCache::Store(const std::string& key,
const std::string& data) { const std::string& data) {
DCHECK(task_runner_->RunsTasksInCurrentSequence()); DCHECK(task_runner_->RunsTasksInCurrentSequence());
base::FilePath subkey_path; base::FilePath subkey_path;
// Delete the file before writing to it. This ensures that the write does not if (!VerifyKeyPathAndGetSubkeyPath(key, true, subkey, &subkey_path))
// follow a symlink planted at |subkey_path|, clobbering a file outside the return false;
// cache directory. The mechanism is meant to foil file-system-level attacks int64_t size = base::checked_cast<int64_t>(data.size());
// where a symlink is planted in the cache directory before Chrome has if (max_cache_size_.has_value() &&
// started. An attacker controlling a process running concurrently with Chrome current_cache_size_ - GetCacheDirectoryOrFileSize(subkey_path) + size >
// would be able to race against the protection by re-creating the symlink max_cache_size_.value()) {
// between these two calls. There is nothing in file_util that could be used LOG(ERROR) << "Data (" << key << ", " << subkey << ") with size " << size
// to protect against such races, especially as the cache is cross-platform << " bytes doesn't fit in cache, left size: "
// and therefore cannot use any POSIX-only tricks. << max_cache_size_.value() - current_cache_size_ << " bytes";
int size = base::checked_cast<int>(data.size()); return false;
return VerifyKeyPathAndGetSubkeyPath(key, true, subkey, &subkey_path) && }
base::DeleteFile(subkey_path, false) && return WriteCacheFile(subkey_path, data);
(base::WriteFile(subkey_path, data.data(), size) == size);
} }
bool ResourceCache::Load(const std::string& key, bool ResourceCache::Load(const std::string& key,
...@@ -115,18 +125,20 @@ void ResourceCache::Delete(const std::string& key, const std::string& subkey) { ...@@ -115,18 +125,20 @@ void ResourceCache::Delete(const std::string& key, const std::string& subkey) {
DCHECK(task_runner_->RunsTasksInCurrentSequence()); DCHECK(task_runner_->RunsTasksInCurrentSequence());
base::FilePath subkey_path; base::FilePath subkey_path;
if (VerifyKeyPathAndGetSubkeyPath(key, false, subkey, &subkey_path)) if (VerifyKeyPathAndGetSubkeyPath(key, false, subkey, &subkey_path))
base::DeleteFile(subkey_path, false); DeleteCacheFile(subkey_path, false);
// Delete() does nothing if the directory given to it is not empty. Hence, the base::FilePath key_path;
// call below deletes the directory representing |key| if its last subkey was // DeleteCacheFile() does nothing if the directory given to it is not empty.
// just removed and does nothing otherwise. // Hence, the call below deletes the directory representing |key| if its last
base::DeleteFile(subkey_path.DirName(), false); // subkey was just removed and does nothing otherwise.
if (VerifyKeyPath(key, false, &key_path))
DeleteCacheFile(key_path, false);
} }
void ResourceCache::Clear(const std::string& key) { void ResourceCache::Clear(const std::string& key) {
DCHECK(task_runner_->RunsTasksInCurrentSequence()); DCHECK(task_runner_->RunsTasksInCurrentSequence());
base::FilePath key_path; base::FilePath key_path;
if (VerifyKeyPath(key, false, &key_path)) if (VerifyKeyPath(key, false, &key_path))
base::DeleteFile(key_path, true); DeleteCacheFile(key_path, true);
} }
void ResourceCache::FilterSubkeys(const std::string& key, void ResourceCache::FilterSubkeys(const std::string& key,
...@@ -147,14 +159,14 @@ void ResourceCache::FilterSubkeys(const std::string& key, ...@@ -147,14 +159,14 @@ void ResourceCache::FilterSubkeys(const std::string& key,
base::Base64UrlDecodePolicy::REQUIRE_PADDING, base::Base64UrlDecodePolicy::REQUIRE_PADDING,
&subkey) || &subkey) ||
subkey.empty() || test.Run(subkey)) { subkey.empty() || test.Run(subkey)) {
base::DeleteFile(subkey_path, true); DeleteCacheFile(subkey_path, true);
} }
} }
// Delete() does nothing if the directory given to it is not empty. Hence, the // Delete() does nothing if the directory given to it is not empty. Hence, the
// call below deletes the directory representing |key| if all of its subkeys // call below deletes the directory representing |key| if all of its subkeys
// were just removed and does nothing otherwise. // were just removed and does nothing otherwise.
base::DeleteFile(key_path, false); DeleteCacheFile(key_path, false);
} }
void ResourceCache::PurgeOtherKeys(const std::set<std::string>& keys_to_keep) { void ResourceCache::PurgeOtherKeys(const std::set<std::string>& keys_to_keep) {
...@@ -169,7 +181,7 @@ void ResourceCache::PurgeOtherKeys(const std::set<std::string>& keys_to_keep) { ...@@ -169,7 +181,7 @@ void ResourceCache::PurgeOtherKeys(const std::set<std::string>& keys_to_keep) {
path = enumerator.Next()) { path = enumerator.Next()) {
const std::string name(path.BaseName().MaybeAsASCII()); const std::string name(path.BaseName().MaybeAsASCII());
if (encoded_keys_to_keep.find(name) == encoded_keys_to_keep.end()) if (encoded_keys_to_keep.find(name) == encoded_keys_to_keep.end())
base::DeleteFile(path, true); DeleteCacheFile(path, true);
} }
} }
...@@ -190,12 +202,12 @@ void ResourceCache::PurgeOtherSubkeys( ...@@ -190,12 +202,12 @@ void ResourceCache::PurgeOtherSubkeys(
path = enumerator.Next()) { path = enumerator.Next()) {
const std::string name(path.BaseName().MaybeAsASCII()); const std::string name(path.BaseName().MaybeAsASCII());
if (encoded_subkeys_to_keep.find(name) == encoded_subkeys_to_keep.end()) if (encoded_subkeys_to_keep.find(name) == encoded_subkeys_to_keep.end())
base::DeleteFile(path, false); DeleteCacheFile(path, false);
} }
// Delete() does nothing if the directory given to it is not empty. Hence, the // Delete() does nothing if the directory given to it is not empty. Hence, the
// call below deletes the directory representing |key| if all of its subkeys // call below deletes the directory representing |key| if all of its subkeys
// were just removed and does nothing otherwise. // were just removed and does nothing otherwise.
base::DeleteFile(key_path, false); DeleteCacheFile(key_path, false);
} }
bool ResourceCache::VerifyKeyPath(const std::string& key, bool ResourceCache::VerifyKeyPath(const std::string& key,
...@@ -236,5 +248,54 @@ bool ResourceCache::VerifyKeyPathAndGetSubkeyPath(const std::string& key, ...@@ -236,5 +248,54 @@ bool ResourceCache::VerifyKeyPathAndGetSubkeyPath(const std::string& key,
return true; return true;
} }
void ResourceCache::InitCurrentCacheSize() {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
current_cache_size_ = GetCacheDirectoryOrFileSize(cache_dir_);
}
bool ResourceCache::WriteCacheFile(const base::FilePath& path,
const std::string& data) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
DCHECK(cache_dir_.IsParent(path));
bool success = DeleteCacheFile(path, false);
int size = base::checked_cast<int>(data.size());
int bytes_written = base::WriteFile(path, data.data(), size);
if (max_cache_size_.has_value())
current_cache_size_ += bytes_written;
return success && bytes_written == size;
}
bool ResourceCache::DeleteCacheFile(const base::FilePath& path,
bool recursive) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
DCHECK(cache_dir_.IsParent(path));
int64_t size = GetCacheDirectoryOrFileSize(path);
bool success = base::DeleteFile(path, recursive);
if (success && max_cache_size_.has_value())
current_cache_size_ -= size;
return success;
}
int64_t ResourceCache::GetCacheDirectoryOrFileSize(
const base::FilePath& path) const {
DCHECK(path == cache_dir_ || cache_dir_.IsParent(path));
if (base::IsLink(path)) {
DLOG(WARNING) << "Symlink " << path.LossyDisplayName()
<< " detected in cache directory";
return 0;
}
int64_t path_size = 0;
if (base::DirectoryExists(path)) {
int types = base::FileEnumerator::FILES | base::FileEnumerator::DIRECTORIES;
base::FileEnumerator enumerator(path, /* recursive */ false, types);
for (base::FilePath path = enumerator.Next(); !path.empty();
path = enumerator.Next()) {
path_size += GetCacheDirectoryOrFileSize(path);
}
} else if (!base::GetFileSize(path, &path_size)) {
path_size = 0;
}
return path_size;
}
} // namespace policy } // namespace policy
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#ifndef COMPONENTS_POLICY_CORE_COMMON_CLOUD_RESOURCE_CACHE_H_ #ifndef COMPONENTS_POLICY_CORE_COMMON_CLOUD_RESOURCE_CACHE_H_
#define COMPONENTS_POLICY_CORE_COMMON_CLOUD_RESOURCE_CACHE_H_ #define COMPONENTS_POLICY_CORE_COMMON_CLOUD_RESOURCE_CACHE_H_
#include <stdint.h>
#include <map> #include <map>
#include <set> #include <set>
#include <string> #include <string>
...@@ -13,6 +15,7 @@ ...@@ -13,6 +15,7 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/optional.h"
#include "components/policy/policy_export.h" #include "components/policy/policy_export.h"
namespace base { namespace base {
...@@ -27,10 +30,13 @@ namespace policy { ...@@ -27,10 +30,13 @@ namespace policy {
// Purge*(). // Purge*().
// The class can be instantiated on any thread but from then on, it must be // The class can be instantiated on any thread but from then on, it must be
// accessed via the |task_runner| only. The |task_runner| must support file I/O. // accessed via the |task_runner| only. The |task_runner| must support file I/O.
// The class needs to have exclusive control on cache directory since it should
// know about all files changes for correct recalculating cache directory size.
class POLICY_EXPORT ResourceCache { class POLICY_EXPORT ResourceCache {
public: public:
explicit ResourceCache(const base::FilePath& cache_path, ResourceCache(const base::FilePath& cache_path,
scoped_refptr<base::SequencedTaskRunner> task_runner); scoped_refptr<base::SequencedTaskRunner> task_runner,
const base::Optional<int64_t> max_cache_size);
virtual ~ResourceCache(); virtual ~ResourceCache();
// Stores |data| under (key, subkey). Returns true if the store suceeded, and // Stores |data| under (key, subkey). Returns true if the store suceeded, and
...@@ -85,11 +91,52 @@ class POLICY_EXPORT ResourceCache { ...@@ -85,11 +91,52 @@ class POLICY_EXPORT ResourceCache {
const std::string& subkey, const std::string& subkey,
base::FilePath* subkey_path); base::FilePath* subkey_path);
// Initializes |current_cache_size_| with the size of cache directory.
// It's called once from constructor and is executed in provided
// |task_runner_|.
void InitCurrentCacheSize();
// Writes the given data into the file in the cache directory and updates
// |current_cache_size_| accordingly.
// Deletes the file before writing to it. This ensures that the write does not
// follow a symlink planted at |subkey_path|, clobbering a file outside the
// cache directory. The mechanism is meant to foil file-system-level attacks
// where a symlink is planted in the cache directory before Chrome has
// started. An attacker controlling a process running concurrently with Chrome
// would be able to race against the protection by re-creating the symlink
// between these two calls. There is nothing in file_util that could be used
// to protect against such races, especially as the cache is cross-platform
// and therefore cannot use any POSIX-only tricks.
// Note that |path| must belong to |cache_dir_|.
bool WriteCacheFile(const base::FilePath& path, const std::string& data);
// Deletes the given path in the cache directory and updates
// |current_cache_size_| accordingly.
// Note that |path| must belong to |cache_dir_|.
bool DeleteCacheFile(const base::FilePath& path, bool recursive);
// Returns the size of given directory or file in the cache directory skipping
// symlinks or 0 if any error occurred.
// We couldn't use |base::ComputeDirectorySize| here because it doesn't allow
// to skip symlinks.
// Skipping symlinks is important here to prevent exploit which puts symlink
// to e.g. root directory and freezes this thread since traversing the whole
// filesystem takes a while.
// Note that |path| must belong to |cache_dir_|.
int64_t GetCacheDirectoryOrFileSize(const base::FilePath& path) const;
base::FilePath cache_dir_; base::FilePath cache_dir_;
// Task runner that |this| runs on. // Task runner that |this| runs on.
scoped_refptr<base::SequencedTaskRunner> task_runner_; scoped_refptr<base::SequencedTaskRunner> task_runner_;
// Maximum size of the cache directory.
const base::Optional<int64_t> max_cache_size_;
// Note that this variable could be created on any thread, but is modified
// only on the |task_runner_| thread.
int64_t current_cache_size_;
DISALLOW_COPY_AND_ASSIGN(ResourceCache); DISALLOW_COPY_AND_ASSIGN(ResourceCache);
}; };
......
...@@ -6,8 +6,14 @@ ...@@ -6,8 +6,14 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/callback.h" #include "base/callback.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/single_thread_task_runner.h"
#include "base/test/scoped_task_environment.h"
#include "base/test/test_simple_task_runner.h" #include "base/test/test_simple_task_runner.h"
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace policy { namespace policy {
...@@ -26,17 +32,32 @@ const char kSubE[] = "eeeee"; ...@@ -26,17 +32,32 @@ const char kSubE[] = "eeeee";
const char kData0[] = "{ \"key\": \"value\" }"; const char kData0[] = "{ \"key\": \"value\" }";
const char kData1[] = "{}"; const char kData1[] = "{}";
const int kMaxCacheSize = 1024 * 10;
const std::string kData1Kb = std::string(1024, ' ');
const std::string kData2Kb = std::string(1024 * 2, ' ');
const std::string kData9Kb = std::string(1024 * 9, ' ');
const std::string kData10Kb = std::string(1024 * 10, ' ');
const std::string kData9KbUpdated = std::string(1024 * 9, '*');
bool Matches(const std::string& expected, const std::string& subkey) { bool Matches(const std::string& expected, const std::string& subkey) {
return subkey == expected; return subkey == expected;
} }
} // namespace } // namespace
TEST(ResourceCacheTest, StoreAndLoad) { class ResourceCacheTest : public testing::Test {
base::ScopedTempDir temp_dir; protected:
ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); }
ResourceCache cache(temp_dir.GetPath(),
base::MakeRefCounted<base::TestSimpleTaskRunner>()); void TearDown() override { task_environment_.RunUntilIdle(); }
base::test::ScopedTaskEnvironment task_environment_;
base::ScopedTempDir temp_dir_;
};
TEST_F(ResourceCacheTest, StoreAndLoad) {
ResourceCache cache(temp_dir_.GetPath(), base::ThreadTaskRunnerHandle::Get(),
/* max_cache_size */ base::nullopt);
// No data initially. // No data initially.
std::string data; std::string data;
...@@ -114,11 +135,9 @@ TEST(ResourceCacheTest, StoreAndLoad) { ...@@ -114,11 +135,9 @@ TEST(ResourceCacheTest, StoreAndLoad) {
EXPECT_EQ(kData1, contents[kSubB]); EXPECT_EQ(kData1, contents[kSubB]);
} }
TEST(ResourceCacheTest, FilterSubkeys) { TEST_F(ResourceCacheTest, FilterSubkeys) {
base::ScopedTempDir temp_dir; ResourceCache cache(temp_dir_.GetPath(), base::ThreadTaskRunnerHandle::Get(),
ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); /* max_cache_size */ base::nullopt);
ResourceCache cache(temp_dir.GetPath(),
base::MakeRefCounted<base::TestSimpleTaskRunner>());
// Store some data. // Store some data.
EXPECT_TRUE(cache.Store(kKey1, kSubA, kData0)); EXPECT_TRUE(cache.Store(kKey1, kSubA, kData0));
...@@ -153,4 +172,75 @@ TEST(ResourceCacheTest, FilterSubkeys) { ...@@ -153,4 +172,75 @@ TEST(ResourceCacheTest, FilterSubkeys) {
EXPECT_EQ(2u, contents.size()); EXPECT_EQ(2u, contents.size());
} }
TEST_F(ResourceCacheTest, StoreWithEnabledCacheLimit) {
ResourceCache cache(temp_dir_.GetPath(), base::ThreadTaskRunnerHandle::Get(),
kMaxCacheSize);
task_environment_.RunUntilIdle();
// Put first subkey with 9Kb data in cache.
EXPECT_TRUE(cache.Store(kKey1, kSubA, kData9Kb));
// Try to put second subkey with 2Kb data in cache, expected to fail while
// total size exceeds 10Kb.
EXPECT_FALSE(cache.Store(kKey2, kSubB, kData2Kb));
// Put second subkey with 1Kb data in cache.
EXPECT_TRUE(cache.Store(kKey2, kSubC, kData1Kb));
// Try to put third subkey with 2 bytes data in cache, expected to fail while
// total size exceeds 10Kb.
EXPECT_FALSE(cache.Store(kKey1, kSubB, kData1));
// Remove keys with all subkeys.
cache.Clear(kKey1);
cache.Clear(kKey2);
// Put first subkey with 9Kb data in cache.
EXPECT_TRUE(cache.Store(kKey3, kSubA, kData9Kb));
// Put second subkey with 1Kb data in cache.
EXPECT_TRUE(cache.Store(kKey3, kSubB, kData1Kb));
// Try to put third subkey with 2 bytes data in cache, expected to fail while
// total size exceeds 10Kb.
EXPECT_FALSE(cache.Store(kKey1, kSubB, kData1));
// Replace data in first subkey with another 9Kb data.
EXPECT_TRUE(cache.Store(kKey3, kSubA, kData9KbUpdated));
// Remove this key with 9Kb data.
cache.Delete(kKey3, kSubA);
// Put second subkey with 2 bytes data in cache.
EXPECT_TRUE(cache.Store(kKey1, kSubB, kData1));
}
#if defined(OS_POSIX) // Because of symbolic links.
TEST_F(ResourceCacheTest, StoreInDirectoryWithCycleSymlinks) {
base::FilePath inner_dir = temp_dir_.GetPath().AppendASCII("inner");
ASSERT_TRUE(base::CreateDirectory(inner_dir));
base::FilePath symlink_to_parent = inner_dir.AppendASCII("symlink");
ASSERT_TRUE(base::CreateSymbolicLink(temp_dir_.GetPath(), symlink_to_parent));
ResourceCache cache(temp_dir_.GetPath(), base::ThreadTaskRunnerHandle::Get(),
kMaxCacheSize);
task_environment_.RunUntilIdle();
// Check if the cache is empty
EXPECT_TRUE(cache.Store(kKey1, kSubA, kData10Kb));
}
TEST_F(ResourceCacheTest, StoreInDirectoryWithSymlinkToRoot) {
base::FilePath inner_dir = temp_dir_.GetPath().AppendASCII("inner");
ASSERT_TRUE(base::CreateDirectory(inner_dir));
base::FilePath root_path(FILE_PATH_LITERAL("/"));
base::FilePath symlink_to_root = temp_dir_.GetPath().AppendASCII("symlink");
ASSERT_TRUE(base::CreateSymbolicLink(root_path, symlink_to_root));
ResourceCache cache(temp_dir_.GetPath(), base::ThreadTaskRunnerHandle::Get(),
kMaxCacheSize);
task_environment_.RunUntilIdle();
// Check if the cache is empty
EXPECT_TRUE(cache.Store(kKey1, kSubA, kData10Kb));
}
#endif // defined(OS_POSIX)
} // namespace policy } // namespace policy
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment