diff options
4 files changed, 33 insertions, 10 deletions
diff --git a/chrome/browser/chromeos/policy/device_local_account_policy_store.cc b/chrome/browser/chromeos/policy/device_local_account_policy_store.cc index 553d5cf..527a5d4 100644 --- a/chrome/browser/chromeos/policy/device_local_account_policy_store.cc +++ b/chrome/browser/chromeos/policy/device_local_account_policy_store.cc @@ -45,6 +45,7 @@ void DeviceLocalAccountPolicyStore::Store( const em::PolicyFetchResponse& policy) { weak_factory_.InvalidateWeakPtrs(); CheckKeyAndValidate( + true, make_scoped_ptr(new em::PolicyFetchResponse(policy)), base::Bind(&DeviceLocalAccountPolicyStore::StoreValidatedPolicy, weak_factory_.GetWeakPtr())); @@ -59,6 +60,7 @@ void DeviceLocalAccountPolicyStore::ValidateLoadedPolicyBlob( scoped_ptr<em::PolicyFetchResponse> policy(new em::PolicyFetchResponse()); if (policy->ParseFromString(policy_blob)) { CheckKeyAndValidate( + false, policy.Pass(), base::Bind(&DeviceLocalAccountPolicyStore::UpdatePolicy, weak_factory_.GetWeakPtr())); @@ -147,16 +149,19 @@ void DeviceLocalAccountPolicyStore::HandleStoreResult(bool success) { } void DeviceLocalAccountPolicyStore::CheckKeyAndValidate( + bool valid_timestamp_required, scoped_ptr<em::PolicyFetchResponse> policy, const UserCloudPolicyValidator::CompletionCallback& callback) { device_settings_service_->GetOwnershipStatusAsync( base::Bind(&DeviceLocalAccountPolicyStore::Validate, weak_factory_.GetWeakPtr(), + valid_timestamp_required, base::Passed(&policy), callback)); } void DeviceLocalAccountPolicyStore::Validate( + bool valid_timestamp_required, scoped_ptr<em::PolicyFetchResponse> policy_response, const UserCloudPolicyValidator::CompletionCallback& callback, chromeos::DeviceSettingsService::OwnershipStatus ownership_status) { @@ -175,9 +180,14 @@ void DeviceLocalAccountPolicyStore::Validate( background_task_runner())); validator->ValidateUsername(account_id_); validator->ValidatePolicyType(dm_protocol::kChromePublicAccountPolicyType); + // The timestamp is verified when storing a new policy downloaded from the + // server but not when loading a cached policy from disk. + // See SessionManagerOperation::ValidateDeviceSettings for the rationale. validator->ValidateAgainstCurrentPolicy( policy(), - CloudPolicyValidatorBase::TIMESTAMP_REQUIRED, + valid_timestamp_required + ? CloudPolicyValidatorBase::TIMESTAMP_REQUIRED + : CloudPolicyValidatorBase::TIMESTAMP_NOT_REQUIRED, CloudPolicyValidatorBase::DM_TOKEN_REQUIRED); validator->ValidatePayload(); validator->ValidateSignature(*key->public_key(), false); diff --git a/chrome/browser/chromeos/policy/device_local_account_policy_store.h b/chrome/browser/chromeos/policy/device_local_account_policy_store.h index 3af5134..1929bd5 100644 --- a/chrome/browser/chromeos/policy/device_local_account_policy_store.h +++ b/chrome/browser/chromeos/policy/device_local_account_policy_store.h @@ -69,11 +69,13 @@ class DeviceLocalAccountPolicyStore // Gets the owner key and triggers policy validation. void CheckKeyAndValidate( + bool valid_timestamp_required, scoped_ptr<enterprise_management::PolicyFetchResponse> policy, const UserCloudPolicyValidator::CompletionCallback& callback); // Triggers policy validation. void Validate( + bool valid_timestamp_required, scoped_ptr<enterprise_management::PolicyFetchResponse> policy, const UserCloudPolicyValidator::CompletionCallback& callback, chromeos::DeviceSettingsService::OwnershipStatus ownership_status); diff --git a/chrome/browser/chromeos/settings/session_manager_operation.cc b/chrome/browser/chromeos/settings/session_manager_operation.cc index c4fc8d1..e3718f9 100644 --- a/chrome/browser/chromeos/settings/session_manager_operation.cc +++ b/chrome/browser/chromeos/settings/session_manager_operation.cc @@ -162,15 +162,23 @@ void SessionManagerOperation::ValidateDeviceSettings( policy::DeviceCloudPolicyValidator::Create(policy.Pass(), background_task_runner); - // Policy auto-generated by session manager doesn't include a timestamp, so we - // need to allow missing timestamps. - const bool require_timestamp = - policy_data_.get() && policy_data_->has_request_token(); + + // Policy auto-generated by session manager doesn't include a timestamp, so + // the timestamp shouldn't be verified in that case. + // + // Additionally, offline devices can get their clock set backwards in time + // under some hardware conditions; checking the timestamp now could likely + // find a value in the future, and prevent the user from signing-in or + // starting guest mode. Tlsdate will eventually fix the clock when the device + // is back online, but the network configuration may come from device ONC. + // + // To prevent all of these issues the timestamp is just not verified when + // loading the device policy from the cache. Note that the timestamp is still + // verified during enrollment and when a new policy is fetched from the + // server. validator->ValidateAgainstCurrentPolicy( policy_data_.get(), - require_timestamp ? - policy::CloudPolicyValidatorBase::TIMESTAMP_REQUIRED : - policy::CloudPolicyValidatorBase::TIMESTAMP_NOT_REQUIRED, + policy::CloudPolicyValidatorBase::TIMESTAMP_NOT_REQUIRED, policy::CloudPolicyValidatorBase::DM_TOKEN_NOT_REQUIRED); validator->ValidatePolicyType(policy::dm_protocol::kChromeDevicePolicyType); validator->ValidatePayload(); diff --git a/chrome/browser/policy/cloud/cloud_policy_validator.cc b/chrome/browser/policy/cloud/cloud_policy_validator.cc index 71649ce..8e148e2 100644 --- a/chrome/browser/policy/cloud/cloud_policy_validator.cc +++ b/chrome/browser/policy/cloud/cloud_policy_validator.cc @@ -276,11 +276,14 @@ CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckTimestamp() { } } - if (policy_data_->timestamp() < timestamp_not_before_) { + if (timestamp_option_ != TIMESTAMP_NOT_REQUIRED && + policy_data_->timestamp() < timestamp_not_before_) { + // If |timestamp_option_| is TIMESTAMP_REQUIRED or TIMESTAMP_NOT_BEFORE + // then this is a failure. LOG(ERROR) << "Policy too old: " << policy_data_->timestamp(); return VALIDATION_BAD_TIMESTAMP; } - if (timestamp_option_ != TIMESTAMP_NOT_BEFORE && + if (timestamp_option_ == TIMESTAMP_REQUIRED && policy_data_->timestamp() > timestamp_not_after_) { LOG(ERROR) << "Policy from the future: " << policy_data_->timestamp(); return VALIDATION_BAD_TIMESTAMP; |