Added support for signed policy blobs on desktop.

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

Index: components/policy/core/common/cloud/user_cloud_policy_store_unittest.cc
diff --git a/components/policy/core/common/cloud/user_cloud_policy_store_unittest.cc b/components/policy/core/common/cloud/user_cloud_policy_store_unittest.cc
index c7378c74806925da7ab9cde9da2e3fa3b82f0aae..1323413746fca2de2da14f246161df990320b54e 100644
--- a/components/policy/core/common/cloud/user_cloud_policy_store_unittest.cc
+++ b/components/policy/core/common/cloud/user_cloud_policy_store_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/run_loop.h"
+#include "components/policy/core/common/cloud/cloud_policy_constants.h"
 #include "components/policy/core/common/cloud/mock_cloud_external_data_manager.h"
 #include "components/policy/core/common/cloud/mock_cloud_policy_store.h"
 #include "components/policy/core/common/cloud/policy_builder.h"
@@ -32,6 +33,8 @@ void RunUntilIdle() {
   run_loop.RunUntilIdle();
 }
 
+}  // namespace
+
 class UserCloudPolicyStoreTest : public testing::Test {
  public:
   UserCloudPolicyStoreTest() {}
@@ -39,15 +42,21 @@ class UserCloudPolicyStoreTest : public testing::Test {
   virtual void SetUp() OVERRIDE {
     ASSERT_TRUE(tmp_dir_.CreateUniqueTempDir());
     store_.reset(
-        new UserCloudPolicyStore(policy_file(), loop_.message_loop_proxy()));
+        new UserCloudPolicyStore(policy_file(),
+                                 key_file(),
+                                 GetPolicyVerificationKey(),
+                                 loop_.message_loop_proxy()));
     external_data_manager_.reset(new MockCloudExternalDataManager);
     external_data_manager_->SetPolicyStore(store_.get());
     store_->SetSigninUsername(PolicyBuilder::kFakeUsername);
     store_->AddObserver(&observer_);
 
-    policy_.payload().mutable_passwordmanagerenabled()->set_value(true);
-    policy_.payload().mutable_urlblacklist()->mutable_value()->add_entries(
-        "chromium.org");
+    // Install an initial public key, so that by default the validation of
+    // the stored/loaded policy blob succeeds (it looks like a new key
+    // provision).
+    policy_.SetDefaultInitialSigningKey();
+
+    InitPolicyPayload(&policy_.payload());
 
     policy_.Build();
   }
@@ -59,10 +68,20 @@ class UserCloudPolicyStoreTest : public testing::Test {
     RunUntilIdle();
   }
 
+  void InitPolicyPayload(enterprise_management::CloudPolicySettings* payload) {
+    payload->mutable_passwordmanagerenabled()->set_value(true);
+    payload->mutable_urlblacklist()->mutable_value()->add_entries(
+        "chromium.org");
+  }
+
   base::FilePath policy_file() {
     return tmp_dir_.path().AppendASCII("policy");
   }
 
+  base::FilePath key_file() {
+    return tmp_dir_.path().AppendASCII("policy_key");
+  }
+
   // Verifies that store_->policy_map() has the appropriate entries.
   void VerifyPolicyMap(CloudPolicyStore* store) {
     EXPECT_EQ(2U, store->policy_map().size());
@@ -162,6 +181,53 @@ TEST_F(UserCloudPolicyStoreTest, LoadImmediatelyWithInvalidFile) {
   EXPECT_TRUE(store_->policy_map().empty());
 }
 
+// Load file from cache with no key data, then migrate to have a key.
+TEST_F(UserCloudPolicyStoreTest, Migration) {
+

[Mattias Nissler (ping if slow), 2014/01/27 13:52:13]

nit: remove newline

[Andrew T Wilson (Slow), 2014/01/30 17:10:31]

Done.

+  UserPolicyBuilder unsigned_builder;
+  unsigned_builder.UnsetSigningKey();
+  InitPolicyPayload(&unsigned_builder.payload());
+  unsigned_builder.Build();
+  // Policy should be unsigned.
+  EXPECT_FALSE(unsigned_builder.policy().has_policy_data_signature());
+
+  // Write policy to disk.
+  std::string data;
+  ASSERT_TRUE(unsigned_builder.policy().SerializeToString(&data));
+  ASSERT_TRUE(base::CreateDirectory(policy_file().DirName()));
+  int size = data.size();
+  ASSERT_EQ(size, file_util::WriteFile(policy_file(), data.c_str(), size));
+
+  // Now make sure the data can get loaded.
+  Sequence s;

[Mattias Nissler (ping if slow), 2014/01/27 13:52:13]

Side note for Bartosz: It seems like testing this external data manager sequence
stuff on each and every test case adds a lot of noise - why don't we have a
single test case that verifies stuff happens in the right order?

[Andrew T Wilson (Slow), 2014/01/30 17:10:31]

Should I do anything to make sure Bartosz sees this comment?

+  EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s);
+  EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s);
+  store_->LoadImmediately();  // Should load without running the message loop.
+  Mock::VerifyAndClearExpectations(external_data_manager_.get());
+  Mock::VerifyAndClearExpectations(&observer_);
+

[Mattias Nissler (ping if slow), 2014/01/27 13:52:13]

nit: remove 2nd blank line

[Andrew T Wilson (Slow), 2014/01/30 17:10:31]

Done.

+
+  ASSERT_TRUE(store_->policy());
+  EXPECT_EQ(unsigned_builder.policy_data().SerializeAsString(),
+            store_->policy()->SerializeAsString());
+  VerifyPolicyMap(store_.get());
+  EXPECT_EQ(CloudPolicyStore::STATUS_OK, store_->status());
+  EXPECT_TRUE(store_->policy_key().empty());
+  EXPECT_FALSE(base::PathExists(key_file()));
+
+  // Now mimic a new policy coming down - this should result in a new key
+  // being installed.
+  {
+    Sequence s;

[Mattias Nissler (ping if slow), 2014/01/27 13:52:13]

can just use a different variable name instead of a nested scope.

[Andrew T Wilson (Slow), 2014/01/30 17:10:31]

Done.

+    EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s);
+    EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s);
+    store_->Store(policy_.policy());
+    RunUntilIdle();
+  }
+  EXPECT_EQ(policy_.policy().new_public_key(), store_->policy_key());
+  EXPECT_TRUE(base::PathExists(key_file()));
+}
+
 TEST_F(UserCloudPolicyStoreTest, Store) {
   EXPECT_FALSE(store_->policy());
   EXPECT_TRUE(store_->policy_map().empty());
@@ -218,6 +284,72 @@ TEST_F(UserCloudPolicyStoreTest, StoreThenClear) {
   EXPECT_EQ(CloudPolicyStore::STATUS_OK, store_->status());
 }
 
+TEST_F(UserCloudPolicyStoreTest, StoreRotatedKey) {
+  EXPECT_FALSE(store_->policy());
+  EXPECT_TRUE(store_->policy_map().empty());
+
+  // Store a simple policy and make sure it ends up as the currently active
+  // policy.
+  {
+    Sequence s;
+    EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s);
+    EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s);
+    store_->Store(policy_.policy());
+    RunUntilIdle();
+    Mock::VerifyAndClearExpectations(external_data_manager_.get());
+    Mock::VerifyAndClearExpectations(&observer_);

[Mattias Nissler (ping if slow), 2014/01/27 13:52:13]

So much boilerplate... can we separate this out into a separate function for
now?

[Andrew T Wilson (Slow), 2014/01/30 17:10:31]

Good call. Done!

+  }
+  ASSERT_TRUE(store_->policy());
+  EXPECT_FALSE(policy_.policy().has_new_public_key_signature());
+  std::string original_policy_key = policy_.policy().new_public_key();
+  EXPECT_EQ(original_policy_key, store_->policy_key());
+
+  // Now do key rotation.
+  policy_.SetDefaultSigningKey();
+  policy_.SetDefaultNewSigningKey();
+  policy_.Build();
+  EXPECT_TRUE(policy_.policy().has_new_public_key_signature());
+  EXPECT_NE(original_policy_key, policy_.policy().new_public_key());
+  {
+    Sequence s;
+    EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s);
+    EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s);
+    store_->Store(policy_.policy());
+    RunUntilIdle();
+  }
+  ASSERT_TRUE(store_->policy());
+  EXPECT_EQ(policy_.policy().new_public_key(), store_->policy_key());
+}
+
+TEST_F(UserCloudPolicyStoreTest, ProvisionKeyTwice) {
+  EXPECT_FALSE(store_->policy());
+  EXPECT_TRUE(store_->policy_map().empty());
+
+  // Store a simple policy and make sure it ends up as the currently active
+  // policy.
+  {
+    Sequence s;
+    EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s);
+    EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s);
+    store_->Store(policy_.policy());
+    RunUntilIdle();
+    Mock::VerifyAndClearExpectations(external_data_manager_.get());
+    Mock::VerifyAndClearExpectations(&observer_);
+  }
+  ASSERT_TRUE(store_->policy());
+
+  // Now try sending down policy signed with a different key (i.e. do key
+  // rotation with a key not signed with the original signing key).
+  policy_.UnsetSigningKey();
+  policy_.SetDefaultNewSigningKey();
+  policy_.Build();
+  EXPECT_FALSE(policy_.policy().has_new_public_key_signature());
+
+  ExpectError(store_.get(), CloudPolicyStore::STATUS_VALIDATION_ERROR);
+  store_->Store(policy_.policy());
+  RunUntilIdle();
+}
+
 TEST_F(UserCloudPolicyStoreTest, StoreTwoTimes) {
   EXPECT_FALSE(store_->policy());
   EXPECT_TRUE(store_->policy_map().empty());
@@ -230,6 +362,7 @@ TEST_F(UserCloudPolicyStoreTest, StoreTwoTimes) {
   EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s1);
 
   UserPolicyBuilder first_policy;
+  first_policy.SetDefaultInitialSigningKey();
   first_policy.payload().mutable_passwordmanagerenabled()->set_value(false);
   first_policy.Build();
   store_->Store(first_policy.policy());
@@ -238,6 +371,11 @@ TEST_F(UserCloudPolicyStoreTest, StoreTwoTimes) {
   Mock::VerifyAndClearExpectations(external_data_manager_.get());
   Mock::VerifyAndClearExpectations(&observer_);
 
+  // Rebuild policy with the same signing key as |first_policy| (no rotation).
+  policy_.UnsetNewSigningKey();
+  policy_.SetDefaultSigningKey();
+  policy_.Build();
+  ASSERT_FALSE(policy_.policy().has_new_public_key());
   Sequence s2;
   EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s2);
   EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s2);
@@ -261,10 +399,14 @@ TEST_F(UserCloudPolicyStoreTest, StoreThenLoad) {
   EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s);
   store_->Store(policy_.policy());
   RunUntilIdle();
+  EXPECT_FALSE(store_->policy_key().empty());
 
   // Now, make sure the policy can be read back in from a second store.
   scoped_ptr<UserCloudPolicyStore> store2(
-      new UserCloudPolicyStore(policy_file(), loop_.message_loop_proxy()));
+      new UserCloudPolicyStore(policy_file(),
+                               key_file(),
+                               GetPolicyVerificationKey(),
+                               loop_.message_loop_proxy()));
   store2->SetSigninUsername(PolicyBuilder::kFakeUsername);
   store2->AddObserver(&observer_);
   EXPECT_CALL(observer_, OnStoreLoaded(store2.get()));
@@ -277,6 +419,8 @@ TEST_F(UserCloudPolicyStoreTest, StoreThenLoad) {
   VerifyPolicyMap(store2.get());
   EXPECT_EQ(CloudPolicyStore::STATUS_OK, store2->status());
   store2->RemoveObserver(&observer_);
+  // Make sure that we properly resurrected the keys.
+  EXPECT_EQ(store2->policy_key(), store_->policy_key());
 }
 
 TEST_F(UserCloudPolicyStoreTest, StoreThenLoadImmediately) {
@@ -290,7 +434,10 @@ TEST_F(UserCloudPolicyStoreTest, StoreThenLoadImmediately) {
 
   // Now, make sure the policy can be read back in from a second store.
   scoped_ptr<UserCloudPolicyStore> store2(
-      new UserCloudPolicyStore(policy_file(), loop_.message_loop_proxy()));
+      new UserCloudPolicyStore(policy_file(),
+                               key_file(),
+                               GetPolicyVerificationKey(),
+                               loop_.message_loop_proxy()));
   store2->SetSigninUsername(PolicyBuilder::kFakeUsername);
   store2->AddObserver(&observer_);
   EXPECT_CALL(observer_, OnStoreLoaded(store2.get()));
@@ -316,18 +463,36 @@ TEST_F(UserCloudPolicyStoreTest, StoreValidationError) {
   ASSERT_FALSE(store_->policy());
 }
 
-TEST_F(UserCloudPolicyStoreTest, LoadValidationError) {
-  // Force a validation error by changing the username after policy is stored.
-  Sequence s;
-  EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s);
-  EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s);
+TEST_F(UserCloudPolicyStoreTest, StoreUnsigned) {
+  // Create unsigned policy, try to store it, should get a validation error.
+  policy_.policy().mutable_policy_data_signature()->clear();
+
+  // Store policy.
+  ExpectError(store_.get(), CloudPolicyStore::STATUS_VALIDATION_ERROR);
   store_->Store(policy_.policy());
   RunUntilIdle();
+  ASSERT_FALSE(store_->policy());
+}
+
+TEST_F(UserCloudPolicyStoreTest, LoadValidationError) {
+  // Force a validation error by changing the username after policy is stored.
+  {
+    Sequence s;
+    EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s);
+    EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s);
+    store_->Store(policy_.policy());
+    RunUntilIdle();
+    Mock::VerifyAndClearExpectations(external_data_manager_.get());
+    Mock::VerifyAndClearExpectations(&observer_);
+  }
 
   // Sign out, and sign back in as a different user, and try to load the profile
   // data (should fail due to mismatched username).
   scoped_ptr<UserCloudPolicyStore> store2(
-      new UserCloudPolicyStore(policy_file(), loop_.message_loop_proxy()));
+      new UserCloudPolicyStore(policy_file(),
+                               key_file(),
+                               GetPolicyVerificationKey(),
+                               loop_.message_loop_proxy()));
   store2->SetSigninUsername("foobar@foobar.com");
   store2->AddObserver(&observer_);
   ExpectError(store2.get(), CloudPolicyStore::STATUS_VALIDATION_ERROR);
@@ -340,7 +505,10 @@ TEST_F(UserCloudPolicyStoreTest, LoadValidationError) {
   // Sign out - we should be able to load the policy (don't check usernames
   // when signed out).
   scoped_ptr<UserCloudPolicyStore> store3(
-      new UserCloudPolicyStore(policy_file(), loop_.message_loop_proxy()));
+      new UserCloudPolicyStore(policy_file(),
+                               key_file(),
+                               GetPolicyVerificationKey(),
+                               loop_.message_loop_proxy()));
   store3->AddObserver(&observer_);
   EXPECT_CALL(observer_, OnStoreLoaded(store3.get()));
   store3->Load();
@@ -351,7 +519,10 @@ TEST_F(UserCloudPolicyStoreTest, LoadValidationError) {
 
   // Now start a signin as a different user - this should fail validation.
   scoped_ptr<UserCloudPolicyStore> store4(
-      new UserCloudPolicyStore(policy_file(), loop_.message_loop_proxy()));
+      new UserCloudPolicyStore(policy_file(),
+                               key_file(),
+                               GetPolicyVerificationKey(),
+                               loop_.message_loop_proxy()));
   store4->SetSigninUsername("foobar@foobar.com");
   store4->AddObserver(&observer_);
   ExpectError(store4.get(), CloudPolicyStore::STATUS_VALIDATION_ERROR);
@@ -362,6 +533,4 @@ TEST_F(UserCloudPolicyStoreTest, LoadValidationError) {
   store4->RemoveObserver(&observer_);
 }
 

[Mattias Nissler (ping if slow), 2014/01/27 13:52:13]

Can you add another test that verifies we detect tampered disk state such as
policy is correctly signed on disk but doesn't check out against the
verification key?

[Andrew T Wilson (Slow), 2014/01/30 17:10:31]

I've added a TODO for this. I intentionally haven't written that test yet - we
don't actually verify signing keys and produce a failure, since the production
server doesn't properly generate a per-domain signature yet for these keys.

I'll add that once I have integrated the proper verification keys from Dennis
and we've updated the signature verification on the server and test servers -
see the TODOs in CloudPolicyValidator.

-}  // namespace
-
 }  // namespace policy