Update policy signature verification to include policy domain.

CloudPolicyValidator now accpets a "domain" parameter which is used to generate
verification signatures for public keys.

Broke out CloudPolicyValidator cached-key verification code into a separate
validation function: ValidateCachedKey().

Added new hard-coded signatures for our PolicyBuilder test keys for the
example.com domain.

BUG=275291
TBR=rogerta@chromium.org

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

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

chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc

@@ -48,7 +48,7 @@ void DeviceCloudPolicyStoreChromeOS::Store(
   scoped_ptr<DeviceCloudPolicyValidator> validator(CreateValidator(policy));
   validator->ValidateSignature(owner_key->public_key_as_string(),
                                GetPolicyVerificationKey(),
-                               std::string(),
+                               install_attributes_->GetDomain(),
                                true);
   validator->ValidateAgainstCurrentPolicy(
       device_settings_service_->policy_data(),
@@ -77,7 +77,8 @@ void DeviceCloudPolicyStoreChromeOS::InstallInitialPolicy(
   }
 
   scoped_ptr<DeviceCloudPolicyValidator> validator(CreateValidator(policy));
-  validator->ValidateInitialKey(GetPolicyVerificationKey());
+  validator->ValidateInitialKey(GetPolicyVerificationKey(),
+                                install_attributes_->GetDomain());
   validator.release()->StartValidation(
       base::Bind(&DeviceCloudPolicyStoreChromeOS::OnPolicyToStoreValidated,
                  weak_factory_.GetWeakPtr()));

chrome/browser/chromeos/policy/device_local_account_policy_store.cc

@@ -7,6 +7,8 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/values.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chromeos/dbus/power_policy_controller.h"
 #include "chromeos/dbus/session_manager_client.h"
 #include "components/policy/core/common/cloud/device_management_service.h"
@@ -190,9 +192,11 @@ void DeviceLocalAccountPolicyStore::Validate(
           : CloudPolicyValidatorBase::TIMESTAMP_NOT_REQUIRED,
       CloudPolicyValidatorBase::DM_TOKEN_REQUIRED);
   validator->ValidatePayload();
+  policy::BrowserPolicyConnectorChromeOS* connector =
+      g_browser_process->platform_part()->browser_policy_connector_chromeos();
   validator->ValidateSignature(key->public_key_as_string(),
                                GetPolicyVerificationKey(),
-                               std::string(),
+                               connector->GetEnterpriseDomain(),
                                false);
   validator.release()->StartValidation(callback);
 }

chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc

@@ -109,13 +109,24 @@ void EnrollmentHandlerChromeOS::OnPolicyFetched(CloudPolicyClient* client) {
 
   validator->ValidateTimestamp(base::Time(), base::Time::NowFromSystemTime(),
                                CloudPolicyValidatorBase::TIMESTAMP_REQUIRED);
-  if (install_attributes_->IsEnterpriseDevice())
-    validator->ValidateDomain(install_attributes_->GetDomain());
+
+  // If this is re-enrollment, make sure that the new policy matches the
+  // previously-enrolled domain.
+  std::string domain;
+  if (install_attributes_->IsEnterpriseDevice()) {
+    domain = install_attributes_->GetDomain();
+    validator->ValidateDomain(domain);
+  }
   validator->ValidateDMToken(client->dm_token(),
                              CloudPolicyValidatorBase::DM_TOKEN_REQUIRED);
   validator->ValidatePolicyType(dm_protocol::kChromeDevicePolicyType);
   validator->ValidatePayload();
-  validator->ValidateInitialKey(GetPolicyVerificationKey());
+  // If |domain| is empty here, the policy validation code will just use the
+  // domain from the username field in the policy itself to do key validation.
+  // TODO(mnissler): Plumb the enrolling user's username into this object so
+  // we can validate the username on the resulting policy, and use the domain
+  // from that username to validate the key below (http://crbug.com/343074).
+  validator->ValidateInitialKey(GetPolicyVerificationKey(), domain);
   validator.release()->StartValidation(
       base::Bind(&EnrollmentHandlerChromeOS::PolicyValidated,
                  weak_ptr_factory_.GetWeakPtr()));

chrome/browser/chromeos/policy/user_cloud_policy_store_chromeos.cc

@@ -49,6 +49,11 @@ void SampleValidationFailure(ValidationFailure sample) {
                             VALIDATION_FAILURE_SIZE);
 }
 
+// Extracts the domain name from the passed username.
+std::string ExtractDomain(const std::string& username) {
+  return gaia::ExtractDomainName(gaia::CanonicalizeEmail(username));
+}
+
 }  // namespace
 
 // Helper class for loading legacy policy caches.
@@ -259,7 +264,7 @@ void UserCloudPolicyStoreChromeOS::LoadImmediately() {
   validator->ValidateSignature(
       policy_key_,
       GetPolicyVerificationKey(),
-      std::string(), // No signature verification needed.
+      ExtractDomain(sanitized_username),
       allow_rotation);
   validator->RunValidation();
   OnRetrievedPolicyValidated(validator.get());
@@ -273,12 +278,13 @@ void UserCloudPolicyStoreChromeOS::ValidatePolicyForStore(
                       CloudPolicyValidatorBase::TIMESTAMP_REQUIRED);
   validator->ValidateUsername(username_);
   if (policy_key_.empty()) {
-    validator->ValidateInitialKey(GetPolicyVerificationKey());
+    validator->ValidateInitialKey(GetPolicyVerificationKey(),
+                                  ExtractDomain(username_));
   } else {
     const bool allow_rotation = true;
     validator->ValidateSignature(policy_key_,
                                  GetPolicyVerificationKey(),
-                                 std::string(),
+                                 ExtractDomain(username_),
                                  allow_rotation);
   }
 
@@ -377,7 +383,7 @@ void UserCloudPolicyStoreChromeOS::ValidateRetrievedPolicy(
   const bool allow_rotation = false;
   validator->ValidateSignature(policy_key_,
                                GetPolicyVerificationKey(),
-                               std::string(),
+                               ExtractDomain(username_),
                                allow_rotation);
   // Start validation. The Validator will delete itself once validation is
   // complete.

chrome/browser/chromeos/policy/user_cloud_policy_store_chromeos_unittest.cc

@@ -41,7 +41,8 @@ namespace {
 
 const char kLegacyDeviceId[] = "legacy-device-id";
 const char kLegacyToken[] = "legacy-token";
-const char kSanitizedUsername[] = "0123456789ABCDEF0123456789ABCDEF012345678";
+const char kSanitizedUsername[] =
+    "0123456789ABCDEF0123456789ABCDEF012345678@example.com";
 const char kDefaultHomepage[] = "http://chromium.org";
 
 ACTION_P2(SendSanitizedUsername, call_status, sanitized_username) {
@@ -599,7 +600,7 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, LoadImmediatelyNoUserPolicyKey) {
       .WillOnce(Return(policy_.GetBlob()));
   EXPECT_CALL(cryptohome_client_,
               BlockingGetSanitizedUsername(PolicyBuilder::kFakeUsername))
-      .WillOnce(Return("wrong"));
+      .WillOnce(Return("wrong@example.com"));
 
   EXPECT_FALSE(store_->policy());
   store_->LoadImmediately();

chrome/browser/chromeos/settings/session_manager_operation.cc

@@ -182,8 +182,10 @@ void SessionManagerOperation::ValidateDeviceSettings(
       policy::CloudPolicyValidatorBase::DM_TOKEN_NOT_REQUIRED);
   validator->ValidatePolicyType(policy::dm_protocol::kChromeDevicePolicyType);
   validator->ValidatePayload();
+  // We don't check the DMServer verification key below, because the signing
+  // key is validated when it is installed.
   validator->ValidateSignature(owner_key_->public_key_as_string(),
-                               policy::GetPolicyVerificationKey(),
+                               std::string(),  // No key validation check.
                                std::string(),
                                false);
   validator->StartValidation(

chrome/browser/chromeos/settings/session_manager_operation_unittest.cc

@@ -271,7 +271,7 @@ TEST_F(SessionManagerOperationTest, SignAndStoreSettings) {
   validator->ValidateSignature(
       public_key_as_string,
       policy::GetPolicyVerificationKey(),
-      policy::PolicyBuilder::GetTestSigningKeySignature(),
+      policy::PolicyBuilder::kFakeDomain,
       false);
   validator->StartValidation(
       base::Bind(&SessionManagerOperationTest::CheckSuccessfulValidation,

components/policy/core/common/cloud/cloud_policy_validator.cc

@@ -113,22 +113,34 @@ void CloudPolicyValidatorBase::ValidatePayload() {
   validation_flags_ |= VALIDATE_PAYLOAD;
 }
 
+
+void CloudPolicyValidatorBase::ValidateCachedKey(
+    const std::string& cached_key,
+    const std::string& cached_key_signature,
+    const std::string& verification_key,
+    const std::string& owning_domain) {
+  validation_flags_ |= VALIDATE_CACHED_KEY;
+  set_verification_key_and_domain(verification_key, owning_domain);
+  cached_key_ = cached_key;
+  cached_key_signature_ = cached_key_signature;
+}
+
 void CloudPolicyValidatorBase::ValidateSignature(
     const std::string& key,
     const std::string& verification_key,
-    const std::string& key_signature,
+    const std::string& owning_domain,
     bool allow_key_rotation) {
   validation_flags_ |= VALIDATE_SIGNATURE;
-  set_verification_key(verification_key);
+  set_verification_key_and_domain(verification_key, owning_domain);
   key_ = key;
-  key_signature_ = key_signature;
   allow_key_rotation_ = allow_key_rotation;
 }
 
 void CloudPolicyValidatorBase::ValidateInitialKey(
-    const std::string& verification_key) {
+    const std::string& verification_key,
+    const std::string& owning_domain) {
   validation_flags_ |= VALIDATE_INITIAL_KEY;
-  set_verification_key(verification_key);
+  set_verification_key_and_domain(verification_key, owning_domain);
 }
 
 void CloudPolicyValidatorBase::ValidateAgainstCurrentPolicy(
@@ -228,6 +240,7 @@ void CloudPolicyValidatorBase::RunChecks() {
   } kCheckFunctions[] = {
     { VALIDATE_SIGNATURE,   &CloudPolicyValidatorBase::CheckSignature },
     { VALIDATE_INITIAL_KEY, &CloudPolicyValidatorBase::CheckInitialKey },
+    { VALIDATE_CACHED_KEY,  &CloudPolicyValidatorBase::CheckCachedKey },
     { VALIDATE_POLICY_TYPE, &CloudPolicyValidatorBase::CheckPolicyType },
     { VALIDATE_ENTITY_ID,   &CloudPolicyValidatorBase::CheckEntityId },
     { VALIDATE_TOKEN,       &CloudPolicyValidatorBase::CheckToken },
@@ -292,16 +305,46 @@ bool CloudPolicyValidatorBase::CheckVerificationKeySignature(
     const std::string& key,
     const std::string& verification_key,
     const std::string& signature) {
-  // TODO(atwilson): Update this routine to include the domain name in the
-  // signed data.
-  return VerifySignature(key, verification_key, signature, SHA256);
+  DCHECK(!verification_key.empty());
+  em::PolicyPublicKeyAndDomain signed_data;
+  signed_data.set_new_public_key(key);
+
+  // If no owning_domain_ supplied, try extracting the domain from the policy
+  // itself (this happens on certain platforms during startup, when we validate
+  // cached policy before prefs are loaded).
+  std::string domain = owning_domain_.empty() ?
+      ExtractDomainFromPolicy() : owning_domain_;
+  if (domain.empty()) {
+    LOG(ERROR) << "Policy does not contain a domain";
+    return false;
+  }
+  signed_data.set_domain(domain);
+  std::string signed_data_as_string;
+  if (!signed_data.SerializeToString(&signed_data_as_string)) {
+    DLOG(ERROR) << "Could not serialize verification key to string";
+    return false;
+  }
+  return VerifySignature(signed_data_as_string, verification_key, signature,
+                         SHA256);
+}
+
+std::string CloudPolicyValidatorBase::ExtractDomainFromPolicy() {
+  std::string domain;
+  if (policy_data_->has_username()) {
+    domain = gaia::ExtractDomainName(
+        gaia::CanonicalizeEmail(
+            gaia::SanitizeEmail(policy_data_->username())));
+  }
+  return domain;
 }
 
-void CloudPolicyValidatorBase::set_verification_key(
-    const std::string& verification_key) {
+void CloudPolicyValidatorBase::set_verification_key_and_domain(
+    const std::string& verification_key, const std::string& owning_domain) {
   // Make sure we aren't overwriting the verification key with a different key.
   DCHECK(verification_key_.empty() || verification_key_ == verification_key);
+  DCHECK(owning_domain_.empty() || owning_domain_ == owning_domain);
   verification_key_ = verification_key;
+  owning_domain_ = owning_domain;
 }
 
 CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckSignature() {
@@ -328,16 +371,6 @@ CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckSignature() {
     return VALIDATION_BAD_SIGNATURE;
   }
 
-  // If a key verification signature is available, then verify the base signing
-  // key as well.
-  if (!key_signature_.empty() && !verification_key_.empty() &&
-      !CheckVerificationKeySignature(key_, verification_key_, key_signature_)) {
-    LOG(ERROR) << "Verification key signature verification failed";
-    return VALIDATION_BAD_KEY_VERIFICATION_SIGNATURE;
-  } else {
-    DVLOG(1) << "Verification key signature verification succeeded";
-  }
-
   return VALIDATION_OK;
 }
 
@@ -357,6 +390,18 @@ CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckInitialKey() {
   return VALIDATION_OK;
 }
 
+CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckCachedKey() {
+  if (!cached_key_signature_.empty() && !verification_key_.empty() &&
+      !CheckVerificationKeySignature(cached_key_, verification_key_,
+                                     cached_key_signature_)) {
+    LOG(ERROR) << "Cached key signature verification failed";
+    return VALIDATION_BAD_KEY_VERIFICATION_SIGNATURE;
+  } else {
+    DVLOG(1) << "Cached key signature verification succeeded";
+  }
+  return VALIDATION_OK;
+}
+
 CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckPolicyType() {
   if (!policy_data_->has_policy_type() ||
        policy_data_->policy_type() != policy_type_) {
@@ -440,18 +485,13 @@ CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckUsername() {
   return VALIDATION_OK;
 }
 
-
 CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckDomain() {
-  if (!policy_data_->has_username()) {
+  std::string policy_domain = ExtractDomainFromPolicy();
+  if (policy_domain.empty()) {
     LOG(ERROR) << "Policy is missing user name";
     return VALIDATION_BAD_USERNAME;
   }
 
-  std::string policy_domain =
-      gaia::ExtractDomainName(
-          gaia::CanonicalizeEmail(
-              gaia::SanitizeEmail(policy_data_->username())));
-
   if (domain_ != policy_domain) {
     LOG(ERROR) << "Invalid user name " << policy_data_->username();
     return VALIDATION_BAD_USERNAME;

components/policy/core/common/cloud/cloud_policy_validator.h

@@ -147,16 +147,23 @@ class POLICY_EXPORT CloudPolicyValidatorBase {
   // Validates that the payload can be decoded successfully.
   void ValidatePayload();
 
+  // Verifies that |cached_key| is valid, by verifying the
+  // |cached_key_signature| using the passed |owning_domain| and
+  // |verification_key|.
+  void ValidateCachedKey(const std::string& cached_key,
+                         const std::string& cached_key_signature,
+                         const std::string& verification_key,
+                         const std::string& owning_domain);
+
   // Verifies that the signature on the policy blob verifies against |key|. If
   // |allow_key_rotation| is true and there is a key rotation present in the
   // policy blob, this checks the signature on the new key against |key| and the
   // policy blob against the new key. New key is also validated using the passed
-  // |verification_key| and the |new_public_key_verification_signature| field.
-  // If |key_signature| is non-empty, then |key| is also verified against that
-  // signature (useful when dealing with cached keys from untrusted sources).
+  // |verification_key| and |owning_domain|, and the
+  // |new_public_key_verification_signature| field.
   void ValidateSignature(const std::string& key,
                          const std::string& verification_key,
-                         const std::string& key_signature,
+                         const std::string& owning_domain,
                          bool allow_key_rotation);
 
   // Similar to ValidateSignature(), this checks the signature on the
@@ -165,7 +172,8 @@ class POLICY_EXPORT CloudPolicyValidatorBase {
   // be called at setup time when there is no existing policy key present to
   // check against. New key is validated using the passed |verification_key| and
   // the new_public_key_verification_signature field.
-  void ValidateInitialKey(const std::string& verification_key);
+  void ValidateInitialKey(const std::string& verification_key,
+                          const std::string& owning_domain);
 
   // Convenience helper that configures timestamp and token validation based on
   // the current policy blob. |policy_data| may be NULL, in which case the
@@ -205,6 +213,7 @@ class POLICY_EXPORT CloudPolicyValidatorBase {
     VALIDATE_PAYLOAD     = 1 << 6,
     VALIDATE_SIGNATURE   = 1 << 7,
     VALIDATE_INITIAL_KEY = 1 << 8,
+    VALIDATE_CACHED_KEY  = 1 << 9,
   };
 
   enum SignatureType {
@@ -236,9 +245,14 @@ class POLICY_EXPORT CloudPolicyValidatorBase {
                                      const std::string& server_key,
                                      const std::string& signature);
 
-  // Sets the key used to verify new public keys, and ensures that callers
-  // don't try to set conflicting keys.
-  void set_verification_key(const std::string& verification_key);
+  // Returns the domain name from the policy being validated. Returns an
+  // empty string if the policy does not contain a username field.
+  std::string ExtractDomainFromPolicy();
+
+  // Sets the key and domain used to verify new public keys, and ensures that
+  // callers don't try to set conflicting values.
+  void set_verification_key_and_domain(const std::string& verification_key,
+                                       const std::string& owning_domain);
 
   // Helper functions implementing individual checks.
   Status CheckTimestamp();
@@ -250,6 +264,7 @@ class POLICY_EXPORT CloudPolicyValidatorBase {
   Status CheckPayload();
   Status CheckSignature();
   Status CheckInitialKey();
+  Status CheckCachedKey();
 
   // Verifies the SHA1/ or SHA256/RSA |signature| on |data| against |key|.
   // |signature_type| specifies the type of signature (SHA1 or SHA256).
@@ -274,8 +289,10 @@ class POLICY_EXPORT CloudPolicyValidatorBase {
   std::string policy_type_;
   std::string settings_entity_id_;
   std::string key_;
-  std::string key_signature_;
+  std::string cached_key_;
+  std::string cached_key_signature_;
   std::string verification_key_;
+  std::string owning_domain_;
   bool allow_key_rotation_;
   scoped_refptr<base::SequencedTaskRunner> background_task_runner_;
 

components/policy/core/common/cloud/cloud_policy_validator_unittest.cc

@@ -16,6 +16,7 @@
 #include "components/policy/core/common/cloud/policy_builder.h"
 #include "components/policy/core/common/policy_switches.h"
 #include "crypto/rsa_private_key.h"
+#include "policy/proto/device_management_backend.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -41,7 +42,8 @@ class CloudPolicyValidatorTest : public testing::Test {
         timestamp_option_(CloudPolicyValidatorBase::TIMESTAMP_REQUIRED),
         ignore_missing_dm_token_(CloudPolicyValidatorBase::DM_TOKEN_REQUIRED),
         allow_key_rotation_(true),
-        existing_dm_token_(PolicyBuilder::kFakeToken) {
+        existing_dm_token_(PolicyBuilder::kFakeToken),
+        owning_domain_(PolicyBuilder::kFakeDomain){
     policy_.SetDefaultNewSigningKey();
   }
 
@@ -51,8 +53,16 @@ class CloudPolicyValidatorTest : public testing::Test {
   }
 
   void Validate(testing::Action<void(UserCloudPolicyValidator*)> check_action) {
+    policy_.Build();
+    ValidatePolicy(check_action, policy_.GetCopy());
+  }
+
+  void ValidatePolicy(
+      testing::Action<void(UserCloudPolicyValidator*)> check_action,
+      scoped_ptr<enterprise_management::PolicyFetchResponse> policy_response) {
     // Create a validator.
-    scoped_ptr<UserCloudPolicyValidator> validator = CreateValidator();
+    scoped_ptr<UserCloudPolicyValidator> validator = CreateValidator(
+        policy_response.Pass());
 
     // Run validation and check the result.
     EXPECT_CALL(*this, ValidationCompletion(validator.get())).WillOnce(
@@ -64,12 +74,12 @@ class CloudPolicyValidatorTest : public testing::Test {
     Mock::VerifyAndClearExpectations(this);
   }
 
-  scoped_ptr<UserCloudPolicyValidator> CreateValidator() {
+  scoped_ptr<UserCloudPolicyValidator> CreateValidator(
+      scoped_ptr<enterprise_management::PolicyFetchResponse> policy_response) {
     std::vector<uint8> public_key_bytes;
     EXPECT_TRUE(
         PolicyBuilder::CreateTestSigningKey()->ExportPublicKey(
             &public_key_bytes));
-    policy_.Build();
 
     // Convert from bytes to string format (which is what ValidateSignature()
     // takes).
@@ -78,20 +88,25 @@ class CloudPolicyValidatorTest : public testing::Test {
         public_key_bytes.size());
 
     UserCloudPolicyValidator* validator = UserCloudPolicyValidator::Create(
-        policy_.GetCopy(), base::MessageLoopProxy::current());
+        policy_response.Pass(), base::MessageLoopProxy::current());
     validator->ValidateTimestamp(timestamp_, timestamp_,
                                  timestamp_option_);
     validator->ValidateUsername(PolicyBuilder::kFakeUsername);
-    validator->ValidateDomain(PolicyBuilder::kFakeDomain);
+    if (!owning_domain_.empty())
+      validator->ValidateDomain(owning_domain_);
     validator->ValidateDMToken(existing_dm_token_, ignore_missing_dm_token_);
     validator->ValidatePolicyType(dm_protocol::kChromeUserPolicyType);
     validator->ValidatePayload();
+    validator->ValidateCachedKey(public_key,
+                                 PolicyBuilder::GetTestSigningKeySignature(),
+                                 GetPolicyVerificationKey(),
+                                 owning_domain_);
     validator->ValidateSignature(public_key,
                                  GetPolicyVerificationKey(),
-                                 PolicyBuilder::GetTestSigningKeySignature(),
+                                 owning_domain_,
                                  allow_key_rotation_);
     if (allow_key_rotation_)
-      validator->ValidateInitialKey(GetPolicyVerificationKey());
+      validator->ValidateInitialKey(GetPolicyVerificationKey(), owning_domain_);
     return make_scoped_ptr(validator);
   }
 
@@ -113,6 +128,7 @@ class CloudPolicyValidatorTest : public testing::Test {
   std::string signing_key_;
   bool allow_key_rotation_;
   std::string existing_dm_token_;
+  std::string owning_domain_;
 
   UserPolicyBuilder policy_;
 
@@ -127,7 +143,9 @@ TEST_F(CloudPolicyValidatorTest, SuccessfulValidation) {
 }
 
 TEST_F(CloudPolicyValidatorTest, SuccessfulRunValidation) {
-  scoped_ptr<UserCloudPolicyValidator> validator = CreateValidator();
+  policy_.Build();
+  scoped_ptr<UserCloudPolicyValidator> validator = CreateValidator(
+      policy_.GetCopy());
   // Run validation immediately (no background tasks).
   validator->RunValidation();
   CheckSuccessfulValidation(validator.get());
@@ -240,7 +258,7 @@ TEST_F(CloudPolicyValidatorTest, ErrorNoUsername) {
 }
 
 TEST_F(CloudPolicyValidatorTest, ErrorInvalidUsername) {
-  policy_.policy_data().set_username("invalid");
+  policy_.policy_data().set_username("invalid@example.com");
   Validate(CheckStatus(CloudPolicyValidatorBase::VALIDATION_BAD_USERNAME));
 }
 
@@ -302,12 +320,42 @@ TEST_F(CloudPolicyValidatorTest, ErrorInvalidPublicKeySignature) {
 // Validation key is not currently checked on Chrome OS
 // (http://crbug.com/328038).
 TEST_F(CloudPolicyValidatorTest, ErrorInvalidPublicKeyVerificationSignature) {
+  policy_.Build();
   policy_.policy().set_new_public_key_verification_signature("invalid");
-  Validate(CheckStatus(
-      CloudPolicyValidatorBase::VALIDATION_BAD_KEY_VERIFICATION_SIGNATURE));
+  ValidatePolicy(CheckStatus(
+      CloudPolicyValidatorBase::VALIDATION_BAD_KEY_VERIFICATION_SIGNATURE),
+                 policy_.GetCopy());
+}
+
+TEST_F(CloudPolicyValidatorTest, ErrorDomainMismatchForKeyVerification) {
+  policy_.Build();
+  // Generate a non-matching owning_domain, which should cause a validation
+  // failure.
+  owning_domain_ = "invalid.com";
+  ValidatePolicy(CheckStatus(
+      CloudPolicyValidatorBase::VALIDATION_BAD_KEY_VERIFICATION_SIGNATURE),
+                 policy_.GetCopy());
+}
+
+TEST_F(CloudPolicyValidatorTest, ErrorDomainExtractedFromUsernameMismatch) {
+  // Generate a non-matching username domain, which should cause a validation
+  // failure when we try to verify the signing key with it.
+  policy_.policy_data().set_username("wonky@invalid.com");
+  policy_.Build();
+  owning_domain_ = "";
+  ValidatePolicy(CheckStatus(
+      CloudPolicyValidatorBase::VALIDATION_BAD_KEY_VERIFICATION_SIGNATURE),
+                 policy_.GetCopy());
 }
 #endif
 
+TEST_F(CloudPolicyValidatorTest, SuccessfulNoDomainValidation) {
+  // Don't pass in a domain - this tells the validation code to instead
+  // extract the domain from the username.
+  owning_domain_ = "";
+  Validate(Invoke(this, &CloudPolicyValidatorTest::CheckSuccessfulValidation));
+}
+
 TEST_F(CloudPolicyValidatorTest, ErrorNoRotationAllowed) {
   allow_key_rotation_ = false;
   Validate(CheckStatus(CloudPolicyValidatorBase::VALIDATION_BAD_SIGNATURE));

components/policy/core/common/cloud/policy_builder.cc

@@ -49,6 +49,32 @@ const uint8 kSigningKey[] = {
     0x98, 0x68, 0xe1, 0x04, 0xa8, 0x92, 0xd0, 0x10, 0xaa,
 };
 
+// SHA256 signature of kSigningKey for "example.com" domain.
+const uint8 kSigningKeySignature[] = {
+    0x97, 0xEB, 0x13, 0xE6, 0x6C, 0xE2, 0x7A, 0x2F, 0xC6, 0x6E, 0x68, 0x8F,
+    0xED, 0x5B, 0x51, 0x08, 0x27, 0xF0, 0xA5, 0x97, 0x20, 0xEE, 0xE2, 0x9B,
+    0x5B, 0x63, 0xA5, 0x9C, 0xAE, 0x41, 0xFD, 0x34, 0xC4, 0x2E, 0xEB, 0x63,
+    0x10, 0x80, 0x0C, 0x74, 0x77, 0x6E, 0x34, 0x1C, 0x1B, 0x3B, 0x8E, 0x2A,
+    0x3A, 0x7F, 0xF9, 0x73, 0xB6, 0x2B, 0xB6, 0x45, 0xDB, 0x05, 0xE8, 0x5A,
+    0x68, 0x36, 0x05, 0x3C, 0x62, 0x3A, 0x6C, 0x64, 0xDB, 0x0E, 0x61, 0xBD,
+    0x29, 0x1C, 0x61, 0x4B, 0xE0, 0xDA, 0x07, 0xBA, 0x29, 0x81, 0xF0, 0x90,
+    0x58, 0xB8, 0xBB, 0xF4, 0x69, 0xFF, 0x8F, 0x2B, 0x4A, 0x2D, 0x98, 0x51,
+    0x37, 0xF5, 0x52, 0xCB, 0xE3, 0xC4, 0x6D, 0xEC, 0xEA, 0x32, 0x2D, 0xDD,
+    0xD7, 0xFC, 0x43, 0xC6, 0x54, 0xE1, 0xC1, 0x66, 0x43, 0x37, 0x09, 0xE1,
+    0xBF, 0xD1, 0x11, 0xFC, 0xDB, 0xBF, 0xDF, 0x66, 0x53, 0x8F, 0x38, 0x2D,
+    0xAA, 0x89, 0xD2, 0x9F, 0x60, 0x90, 0xB7, 0x05, 0xC2, 0x20, 0x82, 0xE6,
+    0xE0, 0x57, 0x55, 0xFF, 0x5F, 0xC1, 0x76, 0x66, 0x46, 0xF8, 0x67, 0xB8,
+    0x8B, 0x81, 0x53, 0xA9, 0x8B, 0x48, 0x9E, 0x2A, 0xF9, 0x60, 0x57, 0xBA,
+    0xD7, 0x52, 0x97, 0x53, 0xF0, 0x2F, 0x78, 0x68, 0x50, 0x18, 0x12, 0x00,
+    0x5E, 0x8E, 0x2A, 0x62, 0x0D, 0x48, 0xA9, 0xB5, 0x6B, 0xBC, 0xA0, 0x52,
+    0x53, 0xD7, 0x65, 0x23, 0xA4, 0xA5, 0xF5, 0x32, 0x49, 0x2D, 0xB2, 0x77,
+    0x2C, 0x66, 0x97, 0xBA, 0x58, 0xE0, 0x16, 0x1C, 0x8C, 0x02, 0x5D, 0xE0,
+    0x73, 0x2E, 0xDF, 0xB4, 0x2F, 0x4C, 0xA2, 0x11, 0x26, 0xC1, 0xAF, 0xAC,
+    0x73, 0xBC, 0xB6, 0x98, 0xE0, 0x20, 0x61, 0x0E, 0x52, 0x4A, 0x6C, 0x80,
+    0xB5, 0x0C, 0x10, 0x80, 0x09, 0x17, 0xF4, 0x9D, 0xFE, 0xB5, 0xFC, 0x63,
+    0x9A, 0x80, 0x3F, 0x76,
+};
+
 // New signing key test data in DER-encoded PKCS8 format.
 const uint8 kNewSigningKey[] = {
     0x30, 0x82, 0x01, 0x54, 0x02, 0x01, 0x00, 0x30, 0x0d, 0x06, 0x09, 0x2a,
@@ -82,6 +108,32 @@ const uint8 kNewSigningKey[] = {
     0x32, 0x1a, 0x6b, 0xb3, 0x5f, 0x35, 0xbd, 0xf3,
 };
 
+// SHA256 signature of kNewSigningKey for "example.com" domain.
+const uint8 kNewSigningKeySignature[] = {
+    0x70, 0xED, 0x27, 0x42, 0x34, 0x69, 0xB6, 0x47, 0x9E, 0x7C, 0xA0, 0xF0,
+    0xE5, 0x0A, 0x49, 0x49, 0x00, 0xDA, 0xBC, 0x70, 0x01, 0xC5, 0x4B, 0xDB,
+    0x47, 0xD5, 0xAF, 0xA1, 0xAD, 0xB7, 0xE4, 0xE1, 0xBD, 0x5A, 0x1C, 0x35,
+    0x44, 0x5A, 0xAA, 0xDB, 0x27, 0xBA, 0xA4, 0xA9, 0xC8, 0xDD, 0xEC, 0xD6,
+    0xEB, 0xFE, 0xDB, 0xE0, 0x03, 0x5C, 0xA6, 0x2E, 0x5A, 0xEC, 0x75, 0x79,
+    0xB8, 0x5F, 0x0A, 0xEE, 0x05, 0xB2, 0x61, 0xDC, 0x58, 0xF0, 0xD1, 0xCB,
+    0x7B, 0x2A, 0xDB, 0xC1, 0x7C, 0x60, 0xE6, 0x3E, 0x87, 0x02, 0x61, 0xE6,
+    0x90, 0xFD, 0x54, 0x65, 0xC7, 0xFF, 0x74, 0x09, 0xD6, 0xAA, 0x8E, 0xDC,
+    0x5B, 0xC8, 0x38, 0x0C, 0x84, 0x0E, 0x84, 0x2E, 0x37, 0x2A, 0x4B, 0xDE,
+    0x31, 0x82, 0x76, 0x1E, 0x77, 0xA5, 0xC1, 0xD5, 0xED, 0xFF, 0xBC, 0xEA,
+    0x91, 0xB7, 0xBC, 0xFF, 0x76, 0x23, 0xE2, 0x78, 0x63, 0x01, 0x47, 0x80,
+    0x47, 0x1F, 0x3A, 0x49, 0xBF, 0x0D, 0xCF, 0x27, 0x70, 0x92, 0xBB, 0xEA,
+    0xB3, 0x92, 0x70, 0xFF, 0x1E, 0x4B, 0x1B, 0xE0, 0x4E, 0x0C, 0x4C, 0x6B,
+    0x5D, 0x77, 0x06, 0xBB, 0xFB, 0x9B, 0x0E, 0x55, 0xB8, 0x8A, 0xF2, 0x45,
+    0xA9, 0xF3, 0x54, 0x3D, 0x0C, 0xAC, 0xA8, 0x15, 0xD2, 0x31, 0x8D, 0x97,
+    0x08, 0x73, 0xC9, 0x0F, 0x1D, 0xDE, 0x10, 0x22, 0xC6, 0x55, 0x53, 0x7F,
+    0x7C, 0x50, 0x16, 0x5A, 0x08, 0xCC, 0x1C, 0x53, 0x9B, 0x02, 0xB8, 0x80,
+    0xB7, 0x46, 0xF5, 0xF1, 0xC7, 0x3D, 0x36, 0xBD, 0x26, 0x02, 0xDE, 0x10,
+    0xAB, 0x5A, 0x03, 0xCD, 0x67, 0x00, 0x1C, 0x23, 0xC7, 0x13, 0xEE, 0x5D,
+    0xAF, 0xC5, 0x1F, 0xE3, 0xA0, 0x54, 0xAC, 0xC2, 0xC9, 0x44, 0xD4, 0x4A,
+    0x09, 0x8E, 0xEB, 0xAE, 0xCA, 0x08, 0x8A, 0x7F, 0x41, 0x7B, 0xD8, 0x2C,
+    0xDD, 0x6F, 0x80, 0xC3,
+};
+
 }  // namespace
 
 // Constants used as dummy data for filling the PolicyData protobuf.
@@ -142,17 +194,20 @@ void PolicyBuilder::SetDefaultNewSigningKey() {
   std::vector<uint8> key(kNewSigningKey,
                          kNewSigningKey + arraysize(kNewSigningKey));
   raw_new_signing_key_.swap(key);
+  raw_new_signing_key_signature_ = GetTestOtherSigningKeySignature();
 }
 
 void PolicyBuilder::SetDefaultInitialSigningKey() {
   std::vector<uint8> key(kSigningKey,
                          kSigningKey + arraysize(kSigningKey));
   raw_new_signing_key_.swap(key);
+  raw_new_signing_key_signature_ = GetTestSigningKeySignature();
   UnsetSigningKey();
 }
 
 void PolicyBuilder::UnsetNewSigningKey() {
   raw_new_signing_key_.clear();
+  raw_new_signing_key_signature_.clear();
 }
 
 void PolicyBuilder::Build() {
@@ -168,6 +223,9 @@ void PolicyBuilder::Build() {
     policy_.set_new_public_key(vector_as_array(&raw_new_public_signing_key),
                                raw_new_public_signing_key.size());
 
+    policy_.set_new_public_key_verification_signature(
+        raw_new_signing_key_signature_);
+
     // The new public key must be signed by the old key.
     scoped_ptr<crypto::RSAPrivateKey> old_signing_key = GetSigningKey();
     if (old_signing_key) {
@@ -218,14 +276,14 @@ scoped_ptr<crypto::RSAPrivateKey> PolicyBuilder::CreateTestOtherSigningKey() {
 
 // static
 std::string PolicyBuilder::GetTestSigningKeySignature() {
-  // TODO(atwilson): Return a real verification signature when one is available.
-  return std::string();
+  return std::string(reinterpret_cast<const char*>(kSigningKeySignature),
+                     sizeof(kSigningKeySignature));
 }
 
 // static
 std::string PolicyBuilder::GetTestOtherSigningKeySignature() {
-  // TODO(atwilson): Return a real verification signature when one is available.
-  return std::string();
+  return std::string(reinterpret_cast<const char*>(kNewSigningKeySignature),
+                     sizeof(kNewSigningKeySignature));
 }
 
 void PolicyBuilder::SignData(const std::string& data,

components/policy/core/common/cloud/policy_builder.h

@@ -89,6 +89,9 @@ class PolicyBuilder {
   static std::string GetTestSigningKeySignature();
   static std::string GetTestOtherSigningKeySignature();
 
+  std::vector<uint8> raw_signing_key() { return raw_signing_key_; }
+  std::vector<uint8> raw_new_signing_key() { return raw_new_signing_key_; }
+
  private:
   // Produces |key|'s signature over |data| and stores it in |signature|.
   void SignData(const std::string& data,
@@ -107,6 +110,7 @@ class PolicyBuilder {
   // temporary RSAPrivateKey is created.
   std::vector<uint8> raw_signing_key_;
   std::vector<uint8> raw_new_signing_key_;
+  std::string raw_new_signing_key_signature_;
 
   DISALLOW_COPY_AND_ASSIGN(PolicyBuilder);
 };

components/policy/core/common/cloud/user_cloud_policy_store.cc

@@ -9,6 +9,7 @@
 #include "base/location.h"
 #include "base/metrics/histogram.h"
 #include "base/task_runner_util.h"
+#include "google_apis/gaia/gaia_auth_util.h"
 #include "policy/proto/cloud_policy.pb.h"
 #include "policy/proto/device_management_backend.pb.h"
 #include "policy/proto/policy_signing_key.pb.h"
@@ -295,10 +296,22 @@ void UserCloudPolicyStore::Validate(
       policy.Pass(),
       CloudPolicyValidatorBase::TIMESTAMP_NOT_BEFORE);
 
-  // Validate the username if the user is signed in.
+  // Extract the owning domain from the signed-in user (if any is set yet).
+  // If there's no owning domain, then the code just ensures that the policy
+  // is self-consistent (that the keys are signed with the same domain that the
+  // username field in the policy contains). UserPolicySigninServerBase will
+  // verify that the username matches the signed in user once profile
+  // initialization is complete (http://crbug.com/342327).
+  std::string owning_domain;
+
+  // Validate the username if the user is signed in. The signin_username_ can
+  // be empty during initial policy load because this happens before the
+  // Prefs subsystem is initialized.
   if (!signin_username_.empty()) {
     DVLOG(1) << "Validating username: " << signin_username_;
     validator->ValidateUsername(signin_username_);
+    owning_domain = gaia::ExtractDomainName(
+        gaia::CanonicalizeEmail(gaia::SanitizeEmail(signin_username_)));
   }
 
   // There are 4 cases:
@@ -325,13 +338,17 @@ void UserCloudPolicyStore::Validate(
       // kMetricPolicyHasVerifiedCachedKey rises to a high enough level.
       DLOG(WARNING) << "Allowing unsigned cached blob for migration";
     } else {
-      // Case #2 - loading from cache with a cached key - just do normal
-      // signature validation using this key. We're loading from cache so don't
-      // allow key rotation.
+      // Case #2 - loading from cache with a cached key - validate the cached
+      // key, then do normal policy data signature validation using the cached
+      // key. We're loading from cache so don't allow key rotation.
+      validator->ValidateCachedKey(cached_key->signing_key(),
+                                   cached_key->signing_key_signature(),
+                                   verification_key_,
+                                   owning_domain);
       const bool no_rotation = false;
       validator->ValidateSignature(cached_key->signing_key(),
                                    verification_key_,
-                                   cached_key->signing_key_signature(),
+                                   owning_domain,
                                    no_rotation);
     }
   } else {
@@ -340,15 +357,15 @@ void UserCloudPolicyStore::Validate(
     if (policy_key_.empty()) {
       // Case #3 - no valid existing policy key, so this new policy fetch should
       // include an initial key provision.
-      validator->ValidateInitialKey(verification_key_);
+      validator->ValidateInitialKey(verification_key_, owning_domain);
     } else {
       // Case #4 - verify new policy with existing key. We always allow key
       // rotation - the verification key will prevent invalid policy from being
-      // injected. |policy_key_| is already known to be valid, so no
-      // verification signature is passed in.
+      // injected. |policy_key_| is already known to be valid, so no need to
+      // verify via ValidateCachedKey().
       const bool allow_rotation = true;
       validator->ValidateSignature(
-          policy_key_, verification_key_, std::string(), allow_rotation);
+          policy_key_, verification_key_, owning_domain, allow_rotation);
     }
   }
 

components/policy/proto/device_management_backend.proto

@@ -297,13 +297,13 @@ message PolicyFetchResponse {
   optional bytes new_public_key_signature = 6;
 
   // If new_public_key is specified, this field contains a signature
-  // of that key, signed using a key only available to DMServer.  
-  // The public key portion of this well-known key is embedded into the
-  // Chrome binary. The hash of that embedded key is passed to DMServer
-  // as verification_key_hash field in PolicyFetchRequest. DMServer will
-  // pick a private key on the server which matches the hash (matches public
-  // key on the client). If DMServer is unable to find matching key, it will
-  // return an error instead of policy data.
+  // of a PolicyPublicKeyAndDomain protobuf, signed using a key only
+  // available to DMServer. The public key portion of this well-known key is
+  // embedded into the Chrome binary. The hash of that embedded key is passed
+  // to DMServer as verification_key_hash field in PolicyFetchRequest. DMServer
+  // will pick a private key on the server which matches the hash (matches
+  // public key on the client). If DMServer is unable to find matching key, it
+  // will return an error instead of policy data.
   // In case hash was not specified, DMServer will leave verification signature
   // field empty (legacy behavior).  
   // In addition to the checks between new_public_key
@@ -313,6 +313,17 @@ message PolicyFetchResponse {
   optional bytes new_public_key_verification_signature = 7;
 }
 
+// Protobuf used to generate the new_public_key_verification_signature field.
+message PolicyPublicKeyAndDomain {
+  // The public key to sign (taken from the |new_public_key| field in
+  // PolicyFetchResponse).
+  optional bytes new_public_key = 1;
+
+  // The domain associated with this key (should match the domain portion of
+  // the username field of the policy).
+  optional string domain = 2;
+}
+
 // Request from device to server for reading policies.
 message DevicePolicyRequest {
   // The policy fetch request.  If this field exists, the request must

google_apis/gaia/gaia_auth_util.cc

@@ -22,10 +22,12 @@ std::string CanonicalizeEmail(const std::string& email_address) {
   std::vector<std::string> parts;
   char at = '@';
   base::SplitString(email_address, at, &parts);
-  if (parts.size() != 2U)
-    NOTREACHED() << "expecting exactly one @, but got " << parts.size();
-  else if (parts[1] == kGmailDomain)  // only strip '.' for gmail accounts.
+  if (parts.size() != 2U) {
+    NOTREACHED() << "expecting exactly one @, but got " << parts.size()-1 <<
+        " : " << email_address;
+  } else if (parts[1] == kGmailDomain) {  // only strip '.' for gmail accounts.
     base::RemoveChars(parts[0], ".", &parts[0]);
+  }
   std::string new_email = StringToLowerASCII(JoinString(parts, at));
   VLOG(1) << "Canonicalized " << email_address << " to " << new_email;
   return new_email;