Fix username validation for device-local accounts policy blobs.

These carry a user name that is determined by the device-local account
device policy configuration, which is an identifier that doesn't
necessarily form a valid email address.

BUG=chromium:333434

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

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

8 files changed, 35 insertions, 18 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 d38305a..7ead08265 100644
--- a/chrome/browser/chromeos/policy/device_local_account_policy_store.cc
+++ b/chrome/browser/chromeos/policy/device_local_account_policy_store.cc
@@ -169,9 +169,11 @@ void DeviceLocalAccountPolicyStore::Validate(
     chromeos::DeviceSettingsService::OwnershipStatus ownership_status) {
   DCHECK_NE(chromeos::DeviceSettingsService::OWNERSHIP_UNKNOWN,
             ownership_status);
+  const em::PolicyData* device_policy_data =
+      device_settings_service_->policy_data();
   scoped_refptr<chromeos::OwnerKey> key =
       device_settings_service_->GetOwnerKey();
-  if (!key.get() || !key->public_key()) {
+  if (!key.get() || !key->public_key() || !device_policy_data) {
     status_ = CloudPolicyStore::STATUS_BAD_STATE;
     NotifyStoreLoaded();
     return;
@@ -180,7 +182,7 @@ void DeviceLocalAccountPolicyStore::Validate(
   scoped_ptr<UserCloudPolicyValidator> validator(
       UserCloudPolicyValidator::Create(policy_response.Pass(),
                                        background_task_runner()));
-  validator->ValidateUsername(account_id_);
+  validator->ValidateUsername(account_id_, false);
   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.
@@ -190,7 +192,12 @@ void DeviceLocalAccountPolicyStore::Validate(
       valid_timestamp_required
           ? CloudPolicyValidatorBase::TIMESTAMP_REQUIRED
           : CloudPolicyValidatorBase::TIMESTAMP_NOT_REQUIRED,
-      CloudPolicyValidatorBase::DM_TOKEN_REQUIRED);
+      CloudPolicyValidatorBase::DM_TOKEN_NOT_REQUIRED);
+
+  // Validate the DMToken to match what device policy has.
+  validator->ValidateDMToken(device_policy_data->request_token(),
+                             CloudPolicyValidatorBase::DM_TOKEN_REQUIRED);
+
   validator->ValidatePayload();
   policy::BrowserPolicyConnectorChromeOS* connector =
       g_browser_process->platform_part()->browser_policy_connector_chromeos();
diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_store_chromeos.cc b/chrome/browser/chromeos/policy/user_cloud_policy_store_chromeos.cc
index afa3d92..a624e33 100644
--- a/chrome/browser/chromeos/policy/user_cloud_policy_store_chromeos.cc
+++ b/chrome/browser/chromeos/policy/user_cloud_policy_store_chromeos.cc
@@ -259,7 +259,7 @@ void UserCloudPolicyStoreChromeOS::LoadImmediately() {
   scoped_ptr<UserCloudPolicyValidator> validator =
       CreateValidator(policy.Pass(),
                       CloudPolicyValidatorBase::TIMESTAMP_REQUIRED);
-  validator->ValidateUsername(username_);
+  validator->ValidateUsername(username_, true);
   const bool allow_rotation = false;
   validator->ValidateSignature(
       policy_key_,
@@ -276,7 +276,7 @@ void UserCloudPolicyStoreChromeOS::ValidatePolicyForStore(
   scoped_ptr<UserCloudPolicyValidator> validator =
       CreateValidator(policy.Pass(),
                       CloudPolicyValidatorBase::TIMESTAMP_REQUIRED);
-  validator->ValidateUsername(username_);
+  validator->ValidateUsername(username_, true);
   if (policy_key_.empty()) {
     validator->ValidateInitialKey(GetPolicyVerificationKey(),
                                   ExtractDomain(username_));
@@ -379,7 +379,7 @@ void UserCloudPolicyStoreChromeOS::ValidateRetrievedPolicy(
   scoped_ptr<UserCloudPolicyValidator> validator =
       CreateValidator(policy.Pass(),
                       CloudPolicyValidatorBase::TIMESTAMP_REQUIRED);
-  validator->ValidateUsername(username_);
+  validator->ValidateUsername(username_, true);
   const bool allow_rotation = false;
   validator->ValidateSignature(policy_key_,
                                GetPolicyVerificationKey(),
@@ -434,7 +434,7 @@ void UserCloudPolicyStoreChromeOS::OnLegacyLoadFinished(
     scoped_ptr<UserCloudPolicyValidator> validator =
         CreateValidator(policy.Pass(),
                         CloudPolicyValidatorBase::TIMESTAMP_REQUIRED);
-    validator->ValidateUsername(username_);
+    validator->ValidateUsername(username_, true);
     validator.release()->StartValidation(
         base::Bind(&UserCloudPolicyStoreChromeOS::OnLegacyPolicyValidated,
                    weak_factory_.GetWeakPtr(),
diff --git a/chrome/browser/chromeos/settings/session_manager_operation_unittest.cc b/chrome/browser/chromeos/settings/session_manager_operation_unittest.cc
index 6c9a6f7..7ae35bc 100644
--- a/chrome/browser/chromeos/settings/session_manager_operation_unittest.cc
+++ b/chrome/browser/chromeos/settings/session_manager_operation_unittest.cc
@@ -254,7 +254,7 @@ TEST_F(SessionManagerOperationTest, SignAndStoreSettings) {
   policy::DeviceCloudPolicyValidator* validator =
       policy::DeviceCloudPolicyValidator::Create(
           policy_response.Pass(), message_loop_.message_loop_proxy());
-  validator->ValidateUsername(policy_.policy_data().username());
+  validator->ValidateUsername(policy_.policy_data().username(), true);
   validator->ValidateTimestamp(
       before,
       after,
diff --git a/components/policy/core/common/cloud/cloud_policy_validator.cc b/components/policy/core/common/cloud/cloud_policy_validator.cc
index 763a4fb..96169eb 100644
--- a/components/policy/core/common/cloud/cloud_policy_validator.cc
+++ b/components/policy/core/common/cloud/cloud_policy_validator.cc
@@ -78,9 +78,11 @@ void CloudPolicyValidatorBase::ValidateTimestamp(
 }
 
 void CloudPolicyValidatorBase::ValidateUsername(
-    const std::string& expected_user) {
+    const std::string& expected_user,
+    bool canonicalize) {
   validation_flags_ |= VALIDATE_USERNAME;
-  user_ = gaia::CanonicalizeEmail(expected_user);
+  user_ = expected_user;
+  canonicalize_user_ = canonicalize;
 }
 
 void CloudPolicyValidatorBase::ValidateDomain(
@@ -172,6 +174,7 @@ CloudPolicyValidatorBase::CloudPolicyValidatorBase(
       timestamp_not_after_(0),
       timestamp_option_(TIMESTAMP_REQUIRED),
       dm_token_option_(DM_TOKEN_REQUIRED),
+      canonicalize_user_(false),
       allow_key_rotation_(false),
       background_task_runner_(background_task_runner) {}
 
@@ -471,10 +474,14 @@ CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckUsername() {
     return VALIDATION_BAD_USERNAME;
   }
 
-  std::string policy_username =
-      gaia::CanonicalizeEmail(gaia::SanitizeEmail(policy_data_->username()));
+  std::string expected = user_;
+  std::string actual = policy_data_->username();
+  if (canonicalize_user_) {
+    expected = gaia::CanonicalizeEmail(gaia::SanitizeEmail(expected));
+    actual = gaia::CanonicalizeEmail(gaia::SanitizeEmail(actual));
+  }
 
-  if (user_ != policy_username) {
+  if (expected != actual) {
     LOG(ERROR) << "Invalid user name " << policy_data_->username();
     return VALIDATION_BAD_USERNAME;
   }
diff --git a/components/policy/core/common/cloud/cloud_policy_validator.h b/components/policy/core/common/cloud/cloud_policy_validator.h
index 3ed2208..9eb781c 100644
--- a/components/policy/core/common/cloud/cloud_policy_validator.h
+++ b/components/policy/core/common/cloud/cloud_policy_validator.h
@@ -125,8 +125,10 @@ class POLICY_EXPORT CloudPolicyValidatorBase {
                          base::Time not_after,
                          ValidateTimestampOption timestamp_option);
 
-  // Validates the username in the policy blob matches |expected_user|.
-  void ValidateUsername(const std::string& expected_user);
+  // Validates that the username in the policy blob matches |expected_user|. If
+  // canonicalize is set to true, both values will be canonicalized before
+  // comparison.
+  void ValidateUsername(const std::string& expected_user, bool canonicalize);
 
   // Validates the policy blob is addressed to |expected_domain|. This uses the
   // domain part of the username field in the policy for the check.
@@ -284,6 +286,7 @@ class POLICY_EXPORT CloudPolicyValidatorBase {
   ValidateTimestampOption timestamp_option_;
   ValidateDMTokenOption dm_token_option_;
   std::string user_;
+  bool canonicalize_user_;
   std::string domain_;
   std::string token_;
   std::string policy_type_;
diff --git a/components/policy/core/common/cloud/cloud_policy_validator_unittest.cc b/components/policy/core/common/cloud/cloud_policy_validator_unittest.cc
index 0c6ef58..7cf4465 100644
--- a/components/policy/core/common/cloud/cloud_policy_validator_unittest.cc
+++ b/components/policy/core/common/cloud/cloud_policy_validator_unittest.cc
@@ -92,7 +92,7 @@ class CloudPolicyValidatorTest : public testing::Test {
         policy_response.Pass(), base::MessageLoopProxy::current());
     validator->ValidateTimestamp(timestamp_, timestamp_,
                                  timestamp_option_);
-    validator->ValidateUsername(PolicyBuilder::kFakeUsername);
+    validator->ValidateUsername(PolicyBuilder::kFakeUsername, true);
     if (!owning_domain_.empty())
       validator->ValidateDomain(owning_domain_);
     validator->ValidateDMToken(existing_dm_token_, ignore_missing_dm_token_);
diff --git a/components/policy/core/common/cloud/component_cloud_policy_store.cc b/components/policy/core/common/cloud/component_cloud_policy_store.cc
index e55688b..3bd1c1c 100644
--- a/components/policy/core/common/cloud/component_cloud_policy_store.cc
+++ b/components/policy/core/common/cloud/component_cloud_policy_store.cc
@@ -279,7 +279,7 @@ bool ComponentCloudPolicyStore::ValidateProto(
   scoped_ptr<ComponentCloudPolicyValidator> validator(
       ComponentCloudPolicyValidator::Create(
           proto.Pass(), scoped_refptr<base::SequencedTaskRunner>()));
-  validator->ValidateUsername(username_);
+  validator->ValidateUsername(username_, true);
   validator->ValidateDMToken(dm_token_,
                              ComponentCloudPolicyValidator::DM_TOKEN_REQUIRED);
   if (!policy_type.empty())
diff --git a/components/policy/core/common/cloud/user_cloud_policy_store.cc b/components/policy/core/common/cloud/user_cloud_policy_store.cc
index 7cd9965..46706eb 100644
--- a/components/policy/core/common/cloud/user_cloud_policy_store.cc
+++ b/components/policy/core/common/cloud/user_cloud_policy_store.cc
@@ -342,7 +342,7 @@ void UserCloudPolicyStore::Validate(
   // Prefs subsystem is initialized.
   if (!signin_username_.empty()) {
     DVLOG(1) << "Validating username: " << signin_username_;
-    validator->ValidateUsername(signin_username_);
+    validator->ValidateUsername(signin_username_, true);
     owning_domain = gaia::ExtractDomainName(
         gaia::CanonicalizeEmail(gaia::SanitizeEmail(signin_username_)));
   }