diff options
8 files changed, 85 insertions, 47 deletions
diff --git a/chrome/browser/policy/device_management_policy_cache.cc b/chrome/browser/policy/device_management_policy_cache.cc index edf27d3..581d998 100644 --- a/chrome/browser/policy/device_management_policy_cache.cc +++ b/chrome/browser/policy/device_management_policy_cache.cc @@ -9,19 +9,22 @@ #include "base/file_util.h" #include "base/logging.h" +#include "base/task.h" #include "base/values.h" #include "chrome/browser/browser_thread.h" +#include "chrome/browser/policy/proto/device_management_local.pb.h" using google::protobuf::RepeatedField; using google::protobuf::RepeatedPtrField; namespace policy { -// A task implementation that saves policy information to a file. +// Saves policy information to a file. class PersistPolicyTask : public Task { public: PersistPolicyTask(const FilePath& path, - const em::DevicePolicyResponse* policy); + const em::DevicePolicyResponse* policy, + const base::Time& timestamp); private: // Task override. @@ -29,17 +32,23 @@ class PersistPolicyTask : public Task { const FilePath path_; scoped_ptr<const em::DevicePolicyResponse> policy_; + const base::Time timestamp_; }; PersistPolicyTask::PersistPolicyTask(const FilePath& path, - const em::DevicePolicyResponse* policy) + const em::DevicePolicyResponse* policy, + const base::Time& timestamp) : path_(path), - policy_(policy) { + policy_(policy), + timestamp_(timestamp) { } void PersistPolicyTask::Run() { std::string data; - if (!policy_->SerializeToString(&data)) { + em::CachedDevicePolicyResponse cached_policy; + cached_policy.mutable_policy()->CopyFrom(*policy_); + cached_policy.set_timestamp(timestamp_.ToInternalValue()); + if (!cached_policy.SerializeToString(&data)) { LOG(WARNING) << "Failed to serialize policy data"; return; } @@ -62,15 +71,6 @@ void DeviceManagementPolicyCache::LoadPolicyFromFile() { if (!file_util::PathExists(backing_file_path_) || fresh_policy_) return; - base::PlatformFileInfo info; - // TODO(danno): The last refresh should actually get stored in the - // persisted cache file. - if (!file_util::GetFileInfo(backing_file_path_, &info)) { - LOG(WARNING) << "Failed to get policy file information from " - << backing_file_path_.value(); - return; - } - // Read the protobuf from the file. std::string data; if (!file_util::ReadFileToString(backing_file_path_, &data)) { @@ -79,20 +79,29 @@ void DeviceManagementPolicyCache::LoadPolicyFromFile() { return; } - em::DevicePolicyResponse policy; - if (!policy.ParseFromArray(data.c_str(), data.size())) { + em::CachedDevicePolicyResponse cached_policy; + if (!cached_policy.ParseFromArray(data.c_str(), data.size())) { LOG(WARNING) << "Failed to parse policy data read from " << backing_file_path_.value(); return; } + // Reject files that claim to be from the future. + base::Time timestamp = base::Time::FromInternalValue( + cached_policy.timestamp()); + if (timestamp > base::Time::NowFromSystemTime()) { + LOG(WARNING) << "Rejected policy data from " << backing_file_path_.value() + << ", file is from the future."; + return; + } + // Decode and swap in the new policy information. - scoped_ptr<DictionaryValue> value(DecodePolicy(policy)); + scoped_ptr<DictionaryValue> value(DecodePolicy(cached_policy.policy())); { AutoLock lock(lock_); if (!fresh_policy_) policy_.reset(value.release()); - last_policy_refresh_time_ = info.last_modified; + last_policy_refresh_time_ = timestamp; } } @@ -112,7 +121,8 @@ void DeviceManagementPolicyCache::SetPolicy( BrowserThread::PostTask( BrowserThread::FILE, FROM_HERE, - new PersistPolicyTask(backing_file_path_, policy_copy)); + new PersistPolicyTask(backing_file_path_, policy_copy, + base::Time::NowFromSystemTime())); } DictionaryValue* DeviceManagementPolicyCache::GetPolicy() { diff --git a/chrome/browser/policy/device_management_policy_cache_unittest.cc b/chrome/browser/policy/device_management_policy_cache_unittest.cc index f3d5cc5..d7df69f 100644 --- a/chrome/browser/policy/device_management_policy_cache_unittest.cc +++ b/chrome/browser/policy/device_management_policy_cache_unittest.cc @@ -5,12 +5,14 @@ #include "chrome/browser/policy/device_management_policy_cache.h" #include <limits> +#include <string> #include "base/file_util.h" #include "base/message_loop.h" #include "base/scoped_temp_dir.h" #include "base/values.h" #include "chrome/browser/browser_thread.h" +#include "chrome/browser/policy/proto/device_management_local.pb.h" #include "testing/gtest/include/gtest/gtest.h" namespace policy { @@ -50,11 +52,15 @@ class DeviceManagementPolicyCacheTest loop_.RunAllPending(); } - void WritePolicy(const em::DevicePolicyResponse& policy) { + void WritePolicy(const em::DevicePolicyResponse& policy, + const base::Time& timestamp) { std::string data; - EXPECT_TRUE(policy.SerializeToString(&data)); - EXPECT_EQ(static_cast<int>(data.size()), - file_util::WriteFile(test_file(), data.c_str(), data.size())); + em::CachedDevicePolicyResponse cached_policy; + cached_policy.mutable_policy()->CopyFrom(policy); + cached_policy.set_timestamp(timestamp.ToInternalValue()); + EXPECT_TRUE(cached_policy.SerializeToString(&data)); + int size = static_cast<int>(data.size()); + EXPECT_EQ(size, file_util::WriteFile(test_file(), data.c_str(), size)); } FilePath test_file() { @@ -75,6 +81,7 @@ TEST_F(DeviceManagementPolicyCacheTest, Empty) { DictionaryValue empty; scoped_ptr<Value> policy(cache.GetPolicy()); EXPECT_TRUE(empty.Equals(policy.get())); + EXPECT_EQ(base::Time(), cache.last_policy_refresh_time()); } TEST_F(DeviceManagementPolicyCacheTest, LoadNoFile) { @@ -83,18 +90,24 @@ TEST_F(DeviceManagementPolicyCacheTest, LoadNoFile) { DictionaryValue empty; scoped_ptr<Value> policy(cache.GetPolicy()); EXPECT_TRUE(empty.Equals(policy.get())); + EXPECT_EQ(base::Time(), cache.last_policy_refresh_time()); } -// Flaky on Windows since the file time is not as precise as the -// system clock time and can end up being later than "now". -// http://crbug.com/62489. -#if defined(OS_WIN) -#define MAYBE_LoadWithFile FLAKY_LoadWithFile -#else -#define MAYBE_LoadWithFile LoadWithFile -#endif -TEST_F(DeviceManagementPolicyCacheTest, MAYBE_LoadWithFile) { - WritePolicy(em::DevicePolicyResponse()); +TEST_F(DeviceManagementPolicyCacheTest, RejectFuture) { + em::DevicePolicyResponse policy_response; + WritePolicy(policy_response, base::Time::NowFromSystemTime() + + base::TimeDelta::FromMinutes(5)); + DeviceManagementPolicyCache cache(test_file()); + cache.LoadPolicyFromFile(); + DictionaryValue empty; + scoped_ptr<Value> policy(cache.GetPolicy()); + EXPECT_TRUE(empty.Equals(policy.get())); + EXPECT_EQ(base::Time(), cache.last_policy_refresh_time()); +} + +TEST_F(DeviceManagementPolicyCacheTest, LoadWithFile) { + em::DevicePolicyResponse policy_response; + WritePolicy(policy_response, base::Time::NowFromSystemTime()); DeviceManagementPolicyCache cache(test_file()); cache.LoadPolicyFromFile(); DictionaryValue empty; @@ -107,7 +120,7 @@ TEST_F(DeviceManagementPolicyCacheTest, MAYBE_LoadWithFile) { TEST_F(DeviceManagementPolicyCacheTest, LoadWithData) { em::DevicePolicyResponse policy; AddStringPolicy(&policy, "HomepageLocation", "http://www.example.com"); - WritePolicy(policy); + WritePolicy(policy, base::Time::NowFromSystemTime()); DeviceManagementPolicyCache cache(test_file()); cache.LoadPolicyFromFile(); DictionaryValue expected; @@ -170,7 +183,7 @@ TEST_F(DeviceManagementPolicyCacheTest, PersistPolicy) { TEST_F(DeviceManagementPolicyCacheTest, FreshPolicyOverride) { em::DevicePolicyResponse policy; AddStringPolicy(&policy, "HomepageLocation", "http://www.example.com"); - WritePolicy(policy); + WritePolicy(policy, base::Time::NowFromSystemTime()); DeviceManagementPolicyCache cache(test_file()); em::DevicePolicyResponse updated_policy; diff --git a/chrome/browser/policy/device_management_policy_provider.cc b/chrome/browser/policy/device_management_policy_provider.cc index b56e37f..babc5a4 100644 --- a/chrome/browser/policy/device_management_policy_provider.cc +++ b/chrome/browser/policy/device_management_policy_provider.cc @@ -8,6 +8,7 @@ #include "base/file_util.h" #include "base/path_service.h" #include "base/task.h" +#include "base/time.h" #include "chrome/browser/browser_thread.h" #include "chrome/browser/policy/device_management_backend.h" #include "chrome/browser/policy/device_management_backend_impl.h" @@ -159,13 +160,13 @@ void DeviceManagementPolicyProvider::SendPolicyRequest() { } bool DeviceManagementPolicyProvider::IsPolicyStale() const { - base::Time now(base::Time::Now()); + base::Time now(base::Time::NowFromSystemTime()); base::Time last_policy_refresh_time = cache_->last_policy_refresh_time(); base::Time policy_expiration_time = last_policy_refresh_time + base::TimeDelta::FromMinutes( kPolicyRefreshRateInMinutes); - return (last_policy_refresh_time > now || now > policy_expiration_time); + return (now > policy_expiration_time); } // static diff --git a/chrome/browser/policy/device_management_policy_provider.h b/chrome/browser/policy/device_management_policy_provider.h index 9fb091a..73a1257 100644 --- a/chrome/browser/policy/device_management_policy_provider.h +++ b/chrome/browser/policy/device_management_policy_provider.h @@ -10,7 +10,6 @@ #include "base/file_path.h" #include "base/scoped_ptr.h" -#include "base/time.h" #include "base/weak_ptr.h" #include "chrome/browser/policy/configuration_policy_provider.h" #include "chrome/browser/policy/device_management_backend.h" diff --git a/chrome/browser/policy/device_management_policy_provider_unittest.cc b/chrome/browser/policy/device_management_policy_provider_unittest.cc index 93b007a..903db44 100644 --- a/chrome/browser/policy/device_management_policy_provider_unittest.cc +++ b/chrome/browser/policy/device_management_policy_provider_unittest.cc @@ -115,17 +115,9 @@ TEST_F(DeviceManagementPolicyProviderTest, EmptyProvideWithFailedBackend) { EXPECT_TRUE(store.policy_map().empty()); } -// Flaky on Windows since the file time is not as precise as the -// system clock time and can end up being later than "now". -// http://crbug.com/62489. -#if defined(OS_WIN) -#define MAYBE_SecondProvide FLAKY_SecondProvide -#else -#define MAYBE_SecondProvide SecondProvide -#endif // If a policy has been fetched previously, if should be available even before // the login succeeds or the device management backend is available. -TEST_F(DeviceManagementPolicyProviderTest, MAYBE_SecondProvide) { +TEST_F(DeviceManagementPolicyProviderTest, SecondProvide) { // Pre-fetch and persist a policy SimulateSuccessfulInitialPolicyFetch(); diff --git a/chrome/browser/policy/proto/device_management_local.proto b/chrome/browser/policy/proto/device_management_local.proto new file mode 100644 index 0000000..cd5a93a --- /dev/null +++ b/chrome/browser/policy/proto/device_management_local.proto @@ -0,0 +1,20 @@ +// Copyright (c) 2010 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. + +syntax = "proto2"; + +option optimize_for = LITE_RUNTIME; + +package enterprise_management; + +import "device_management_backend.proto"; + +// Wrapper around DevicePolicyResponse for caching on disk. +message CachedDevicePolicyResponse { + // The DevicePolicyResponse wrapped by this message. + required DevicePolicyResponse policy = 1; + // Timestamp noting when this policy was cached. + optional uint64 timestamp = 2; +} + diff --git a/chrome/browser/policy/proto/device_management_proto.gyp b/chrome/browser/policy/proto/device_management_proto.gyp index c8b61e0..0fc6f82 100644 --- a/chrome/browser/policy/proto/device_management_proto.gyp +++ b/chrome/browser/policy/proto/device_management_proto.gyp @@ -14,6 +14,7 @@ 'type': 'none', 'sources': [ 'device_management_backend.proto', + 'device_management_local.proto', ], 'rules': [ { diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index e17ae8c9..6710a08 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -2417,6 +2417,8 @@ # TODO(danno): Find a better way to include these files '<(protoc_out_dir)/chrome/browser/policy/proto/device_management_backend.pb.cc', '<(protoc_out_dir)/chrome/browser/policy/proto/device_management_backend.pb.h', + '<(protoc_out_dir)/chrome/browser/policy/proto/device_management_local.pb.cc', + '<(protoc_out_dir)/chrome/browser/policy/proto/device_management_local.pb.h', 'browser/popup_blocked_animation.h', 'browser/possible_url_model.cc', 'browser/possible_url_model.h', |