Don't verify the policy timestamp when loading from cache.

There are conditions that cause a device's clock to go back in time, and then
if the policy is rejected because its timestamp is in the future then no user
can sign-in, and guest sessions can't be started.

This CL removes the timestamp check from policies loaded from the cache;
new policies downloaded from the server or downloaded during enrollment still
have their timestamps checked.

BUG=313906,265507,303508
R=bartfab@chromium.org, xiyuan@chromium.org

Review URL: https://codereview.chromium.org/62453002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@233323 0039d316-1c4b-4281-b951-d872f2087c98

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);

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);

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();

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;