Commit 8d8a9491 authored by atwilson@chromium.org's avatar atwilson@chromium.org

Turn off future-timestamp cloud policy checks on desktop

BUG=279099

Review URL: https://chromiumcodereview.appspot.com/24041002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@222028 0039d316-1c4b-4281-b951-d872f2087c98
parent 5ba2fa14
...@@ -243,7 +243,8 @@ void UserCloudPolicyStoreChromeOS::LoadImmediately() { ...@@ -243,7 +243,8 @@ void UserCloudPolicyStoreChromeOS::LoadImmediately() {
policy_key_loaded_ = true; policy_key_loaded_ = true;
scoped_ptr<UserCloudPolicyValidator> validator = scoped_ptr<UserCloudPolicyValidator> validator =
CreateValidator(policy.Pass()); CreateValidator(policy.Pass(),
CloudPolicyValidatorBase::TIMESTAMP_REQUIRED);
validator->ValidateUsername(username_); validator->ValidateUsername(username_);
const bool allow_rotation = false; const bool allow_rotation = false;
validator->ValidateSignature(policy_key_, allow_rotation); validator->ValidateSignature(policy_key_, allow_rotation);
...@@ -255,7 +256,8 @@ void UserCloudPolicyStoreChromeOS::ValidatePolicyForStore( ...@@ -255,7 +256,8 @@ void UserCloudPolicyStoreChromeOS::ValidatePolicyForStore(
scoped_ptr<em::PolicyFetchResponse> policy) { scoped_ptr<em::PolicyFetchResponse> policy) {
// Create and configure a validator. // Create and configure a validator.
scoped_ptr<UserCloudPolicyValidator> validator = scoped_ptr<UserCloudPolicyValidator> validator =
CreateValidator(policy.Pass()); CreateValidator(policy.Pass(),
CloudPolicyValidatorBase::TIMESTAMP_REQUIRED);
validator->ValidateUsername(username_); validator->ValidateUsername(username_);
if (policy_key_.empty()) { if (policy_key_.empty()) {
validator->ValidateInitialKey(); validator->ValidateInitialKey();
...@@ -353,7 +355,8 @@ void UserCloudPolicyStoreChromeOS::ValidateRetrievedPolicy( ...@@ -353,7 +355,8 @@ void UserCloudPolicyStoreChromeOS::ValidateRetrievedPolicy(
scoped_ptr<em::PolicyFetchResponse> policy) { scoped_ptr<em::PolicyFetchResponse> policy) {
// Create and configure a validator for the loaded policy. // Create and configure a validator for the loaded policy.
scoped_ptr<UserCloudPolicyValidator> validator = scoped_ptr<UserCloudPolicyValidator> validator =
CreateValidator(policy.Pass()); CreateValidator(policy.Pass(),
CloudPolicyValidatorBase::TIMESTAMP_REQUIRED);
validator->ValidateUsername(username_); validator->ValidateUsername(username_);
const bool allow_rotation = false; const bool allow_rotation = false;
validator->ValidateSignature(policy_key_, allow_rotation); validator->ValidateSignature(policy_key_, allow_rotation);
...@@ -404,7 +407,8 @@ void UserCloudPolicyStoreChromeOS::OnLegacyLoadFinished( ...@@ -404,7 +407,8 @@ void UserCloudPolicyStoreChromeOS::OnLegacyLoadFinished(
// Create and configure a validator for the loaded legacy policy. Note that // Create and configure a validator for the loaded legacy policy. Note that
// the signature on this policy is not verified. // the signature on this policy is not verified.
scoped_ptr<UserCloudPolicyValidator> validator = scoped_ptr<UserCloudPolicyValidator> validator =
CreateValidator(policy.Pass()); CreateValidator(policy.Pass(),
CloudPolicyValidatorBase::TIMESTAMP_REQUIRED);
validator->ValidateUsername(username_); validator->ValidateUsername(username_);
validator.release()->StartValidation( validator.release()->StartValidation(
base::Bind(&UserCloudPolicyStoreChromeOS::OnLegacyPolicyValidated, base::Bind(&UserCloudPolicyStoreChromeOS::OnLegacyPolicyValidated,
......
...@@ -278,7 +278,8 @@ CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckTimestamp() { ...@@ -278,7 +278,8 @@ CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckTimestamp() {
LOG(ERROR) << "Policy too old: " << policy_data_->timestamp(); LOG(ERROR) << "Policy too old: " << policy_data_->timestamp();
return VALIDATION_BAD_TIMESTAMP; return VALIDATION_BAD_TIMESTAMP;
} }
if (policy_data_->timestamp() > timestamp_not_after_) { if (timestamp_option_ != TIMESTAMP_NOT_BEFORE &&
policy_data_->timestamp() > timestamp_not_after_) {
LOG(ERROR) << "Policy from the future: " << policy_data_->timestamp(); LOG(ERROR) << "Policy from the future: " << policy_data_->timestamp();
return VALIDATION_BAD_TIMESTAMP; return VALIDATION_BAD_TIMESTAMP;
} }
......
...@@ -79,9 +79,16 @@ class CloudPolicyValidatorBase { ...@@ -79,9 +79,16 @@ class CloudPolicyValidatorBase {
}; };
enum ValidateTimestampOption { enum ValidateTimestampOption {
// The policy must have a timestamp field. // The policy must have a timestamp field and it should be checked against
// both the start and end times.
TIMESTAMP_REQUIRED, TIMESTAMP_REQUIRED,
// The timestamp should only be compared vs the |not_before| value (this
// is appropriate for platforms with unreliable system times, where we want
// to ensure that fresh policy is newer than existing policy, but we can't
// do any other validation).
TIMESTAMP_NOT_BEFORE,
// No timestamp field is required. // No timestamp field is required.
TIMESTAMP_NOT_REQUIRED, TIMESTAMP_NOT_REQUIRED,
}; };
...@@ -102,11 +109,11 @@ class CloudPolicyValidatorBase { ...@@ -102,11 +109,11 @@ class CloudPolicyValidatorBase {
} }
// Instructs the validator to check that the policy timestamp is not before // Instructs the validator to check that the policy timestamp is not before
// |not_before| and not after |now| + grace interval. If // |not_before| and not after |not_after| + grace interval. If
// |timestamp_option| is set to TIMESTAMP_REQUIRED, then the policy will fail // |timestamp_option| is set to TIMESTAMP_REQUIRED, then the policy will fail
// validation if it does not have a timestamp field. // validation if it does not have a timestamp field.
void ValidateTimestamp(base::Time not_before, void ValidateTimestamp(base::Time not_before,
base::Time now, base::Time not_after,
ValidateTimestampOption timestamp_option); ValidateTimestampOption timestamp_option);
// Validates the username in the policy blob matches |expected_user|. // Validates the username in the policy blob matches |expected_user|.
......
...@@ -36,7 +36,7 @@ class CloudPolicyValidatorTest : public testing::Test { ...@@ -36,7 +36,7 @@ class CloudPolicyValidatorTest : public testing::Test {
timestamp_(base::Time::UnixEpoch() + timestamp_(base::Time::UnixEpoch() +
base::TimeDelta::FromMilliseconds( base::TimeDelta::FromMilliseconds(
PolicyBuilder::kFakeTimestamp)), PolicyBuilder::kFakeTimestamp)),
ignore_missing_timestamp_(CloudPolicyValidatorBase::TIMESTAMP_REQUIRED), timestamp_option_(CloudPolicyValidatorBase::TIMESTAMP_REQUIRED),
ignore_missing_dm_token_(CloudPolicyValidatorBase::DM_TOKEN_REQUIRED), ignore_missing_dm_token_(CloudPolicyValidatorBase::DM_TOKEN_REQUIRED),
allow_key_rotation_(true), allow_key_rotation_(true),
existing_dm_token_(PolicyBuilder::kFakeToken), existing_dm_token_(PolicyBuilder::kFakeToken),
...@@ -67,7 +67,7 @@ class CloudPolicyValidatorTest : public testing::Test { ...@@ -67,7 +67,7 @@ class CloudPolicyValidatorTest : public testing::Test {
UserCloudPolicyValidator* validator = UserCloudPolicyValidator* validator =
UserCloudPolicyValidator::Create(policy_.GetCopy()); UserCloudPolicyValidator::Create(policy_.GetCopy());
validator->ValidateTimestamp(timestamp_, timestamp_, validator->ValidateTimestamp(timestamp_, timestamp_,
ignore_missing_timestamp_); timestamp_option_);
validator->ValidateUsername(PolicyBuilder::kFakeUsername); validator->ValidateUsername(PolicyBuilder::kFakeUsername);
validator->ValidateDomain(PolicyBuilder::kFakeDomain); validator->ValidateDomain(PolicyBuilder::kFakeDomain);
validator->ValidateDMToken(existing_dm_token_, ignore_missing_dm_token_); validator->ValidateDMToken(existing_dm_token_, ignore_missing_dm_token_);
...@@ -92,7 +92,7 @@ class CloudPolicyValidatorTest : public testing::Test { ...@@ -92,7 +92,7 @@ class CloudPolicyValidatorTest : public testing::Test {
base::MessageLoop loop_; base::MessageLoop loop_;
base::Time timestamp_; base::Time timestamp_;
CloudPolicyValidatorBase::ValidateTimestampOption ignore_missing_timestamp_; CloudPolicyValidatorBase::ValidateTimestampOption timestamp_option_;
CloudPolicyValidatorBase::ValidateDMTokenOption ignore_missing_dm_token_; CloudPolicyValidatorBase::ValidateDMTokenOption ignore_missing_dm_token_;
std::string signing_key_; std::string signing_key_;
bool allow_key_rotation_; bool allow_key_rotation_;
...@@ -153,7 +153,7 @@ TEST_F(CloudPolicyValidatorTest, ErrorNoTimestamp) { ...@@ -153,7 +153,7 @@ TEST_F(CloudPolicyValidatorTest, ErrorNoTimestamp) {
} }
TEST_F(CloudPolicyValidatorTest, IgnoreMissingTimestamp) { TEST_F(CloudPolicyValidatorTest, IgnoreMissingTimestamp) {
ignore_missing_timestamp_ = CloudPolicyValidatorBase::TIMESTAMP_NOT_REQUIRED; timestamp_option_ = CloudPolicyValidatorBase::TIMESTAMP_NOT_REQUIRED;
policy_.policy_data().clear_timestamp(); policy_.policy_data().clear_timestamp();
Validate(CheckStatus(CloudPolicyValidatorBase::VALIDATION_OK)); Validate(CheckStatus(CloudPolicyValidatorBase::VALIDATION_OK));
} }
...@@ -172,6 +172,15 @@ TEST_F(CloudPolicyValidatorTest, ErrorTimestampFromTheFuture) { ...@@ -172,6 +172,15 @@ TEST_F(CloudPolicyValidatorTest, ErrorTimestampFromTheFuture) {
Validate(CheckStatus(CloudPolicyValidatorBase::VALIDATION_BAD_TIMESTAMP)); Validate(CheckStatus(CloudPolicyValidatorBase::VALIDATION_BAD_TIMESTAMP));
} }
TEST_F(CloudPolicyValidatorTest, IgnoreErrorTimestampFromTheFuture) {
base::Time timestamp(timestamp_ + base::TimeDelta::FromMinutes(5));
timestamp_option_ =
CloudPolicyValidatorBase::TIMESTAMP_NOT_BEFORE;
policy_.policy_data().set_timestamp(
(timestamp - base::Time::UnixEpoch()).InMilliseconds());
Validate(CheckStatus(CloudPolicyValidatorBase::VALIDATION_OK));
}
TEST_F(CloudPolicyValidatorTest, ErrorNoRequestToken) { TEST_F(CloudPolicyValidatorTest, ErrorNoRequestToken) {
policy_.policy_data().clear_request_token(); policy_.policy_data().clear_request_token();
Validate(CheckStatus(CloudPolicyValidatorBase::VALIDATION_WRONG_TOKEN)); Validate(CheckStatus(CloudPolicyValidatorBase::VALIDATION_WRONG_TOKEN));
......
...@@ -210,8 +210,9 @@ void UserCloudPolicyStore::Validate( ...@@ -210,8 +210,9 @@ void UserCloudPolicyStore::Validate(
bool validate_in_background, bool validate_in_background,
const UserCloudPolicyValidator::CompletionCallback& callback) { const UserCloudPolicyValidator::CompletionCallback& callback) {
// Configure the validator. // Configure the validator.
scoped_ptr<UserCloudPolicyValidator> validator = scoped_ptr<UserCloudPolicyValidator> validator = CreateValidator(
CreateValidator(policy.Pass()); policy.Pass(),
CloudPolicyValidatorBase::TIMESTAMP_NOT_BEFORE);
SigninManager* signin = SigninManagerFactory::GetForProfileIfExists(profile_); SigninManager* signin = SigninManagerFactory::GetForProfileIfExists(profile_);
if (signin) { if (signin) {
std::string username = signin->GetAuthenticatedUsername(); std::string username = signin->GetAuthenticatedUsername();
......
...@@ -24,14 +24,15 @@ UserCloudPolicyStoreBase::~UserCloudPolicyStoreBase() { ...@@ -24,14 +24,15 @@ UserCloudPolicyStoreBase::~UserCloudPolicyStoreBase() {
} }
scoped_ptr<UserCloudPolicyValidator> UserCloudPolicyStoreBase::CreateValidator( scoped_ptr<UserCloudPolicyValidator> UserCloudPolicyStoreBase::CreateValidator(
scoped_ptr<enterprise_management::PolicyFetchResponse> policy) { scoped_ptr<enterprise_management::PolicyFetchResponse> policy,
CloudPolicyValidatorBase::ValidateTimestampOption timestamp_option) {
// Configure the validator. // Configure the validator.
UserCloudPolicyValidator* validator = UserCloudPolicyValidator* validator =
UserCloudPolicyValidator::Create(policy.Pass()); UserCloudPolicyValidator::Create(policy.Pass());
validator->ValidatePolicyType(GetChromeUserPolicyType()); validator->ValidatePolicyType(GetChromeUserPolicyType());
validator->ValidateAgainstCurrentPolicy( validator->ValidateAgainstCurrentPolicy(
policy_.get(), policy_.get(),
CloudPolicyValidatorBase::TIMESTAMP_REQUIRED, timestamp_option,
CloudPolicyValidatorBase::DM_TOKEN_REQUIRED); CloudPolicyValidatorBase::DM_TOKEN_REQUIRED);
validator->ValidatePayload(); validator->ValidatePayload();
return scoped_ptr<UserCloudPolicyValidator>(validator); return scoped_ptr<UserCloudPolicyValidator>(validator);
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "chrome/browser/policy/cloud/cloud_policy_store.h" #include "chrome/browser/policy/cloud/cloud_policy_store.h"
#include "chrome/browser/policy/cloud/cloud_policy_validator.h"
namespace policy { namespace policy {
...@@ -25,7 +26,8 @@ class UserCloudPolicyStoreBase : public CloudPolicyStore { ...@@ -25,7 +26,8 @@ class UserCloudPolicyStoreBase : public CloudPolicyStore {
// Creates a validator configured to validate a user policy. The caller owns // Creates a validator configured to validate a user policy. The caller owns
// the resulting object until StartValidation() is invoked. // the resulting object until StartValidation() is invoked.
scoped_ptr<UserCloudPolicyValidator> CreateValidator( scoped_ptr<UserCloudPolicyValidator> CreateValidator(
scoped_ptr<enterprise_management::PolicyFetchResponse> policy); scoped_ptr<enterprise_management::PolicyFetchResponse> policy,
CloudPolicyValidatorBase::ValidateTimestampOption option);
// Sets |policy_data| and |payload| as the active policy. // Sets |policy_data| and |payload| as the active policy.
void InstallPolicy( void InstallPolicy(
......
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