Don't upload extension IDs in the cloud policy protocol.

The cloud policy protocol can also be used to fetch policy for extensions; this change makes an update to the protocol such that extension IDs are never uploaded to the server.

Note that this feature is still disabled by default behind the --enable-component-cloud-policy flag.

BUG=361156

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

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

chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc

@@ -54,6 +54,8 @@ namespace em = enterprise_management;
 
 namespace policy {
 
+namespace {
+
 const char kDMToken[] = "dmtoken";
 const char kDeviceID[] = "deviceid";
 
@@ -84,8 +86,9 @@ const char kTestPolicy2[] =
 
 const char kTestPolicy2JSON[] = "{\"Another\":\"turn_it_off\"}";
 
+#if !defined(OS_CHROMEOS)
 // Same encoding as ResourceCache does for its keys.
-bool Base64Encode(const std::string& value, std::string* encoded) {
+bool Base64UrlEncode(const std::string& value, std::string* encoded) {
   if (value.empty())
     return false;
   base::Base64Encode(value, encoded);
@@ -93,6 +96,9 @@ bool Base64Encode(const std::string& value, std::string* encoded) {
   base::ReplaceChars(*encoded, "/", "_", encoded);
   return true;
 }
+#endif
+
+}  // namespace
 
 class ComponentCloudPolicyTest : public ExtensionBrowserTest {
  protected:
@@ -294,9 +300,9 @@ IN_PROC_BROWSER_TEST_F(ComponentCloudPolicyTest, SignOutAndBackIn) {
 
   // Verify that the policy cache exists.
   std::string cache_key;
-  ASSERT_TRUE(Base64Encode("extension-policy", &cache_key));
+  ASSERT_TRUE(Base64UrlEncode("extension-policy", &cache_key));
   std::string cache_subkey;
-  ASSERT_TRUE(Base64Encode(kTestExtension, &cache_subkey));
+  ASSERT_TRUE(Base64UrlEncode(kTestExtension, &cache_subkey));
   base::FilePath cache_path = browser()->profile()->GetPath()
       .Append(FILE_PATH_LITERAL("Policy"))
       .Append(FILE_PATH_LITERAL("Components"))

chrome/browser/policy/test/policy_testserver.py

@@ -57,6 +57,7 @@ Example:
 import base64
 import BaseHTTPServer
 import cgi
+import glob
 import google.protobuf.text_format
 import hashlib
 import logging
@@ -277,8 +278,7 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     # device_management_backend.proto.
     if (self.GetUniqueParam('devicetype') != '2' or
         self.GetUniqueParam('apptype') != 'Chrome' or
-        (request_type != 'ping' and
-         len(self.GetUniqueParam('deviceid')) >= 64) or
+        len(self.GetUniqueParam('deviceid')) >= 64 or
         len(self.GetUniqueParam('agent')) >= 64):
       return (400, 'Invalid request parameter')
     if request_type == 'register':
@@ -287,7 +287,7 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
       response = self.ProcessApiAuthorization(rmsg.service_api_access_request)
     elif request_type == 'unregister':
       response = self.ProcessUnregister(rmsg.unregister_request)
-    elif request_type == 'policy' or request_type == 'ping':
+    elif request_type == 'policy':
       response = self.ProcessPolicy(rmsg, request_type)
     elif request_type == 'enterprise_check':
       response = self.ProcessAutoEnrollment(rmsg.auto_enrollment_request)
@@ -308,7 +308,7 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     Otherwise, the protobuf will be empty.
 
     Args:
-      policy_key: the policy type and settings entity id, joined by '/'.
+      policy_key: The policy type and settings entity id, joined by '/'.
 
     Returns:
       A serialized ExternalPolicyData.
@@ -445,20 +445,18 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
 
     response = dm.DeviceManagementResponse()
     for request in msg.policy_request.request:
-      fetch_response = response.policy_response.response.add()
       if (request.policy_type in
              ('google/android/user',
-              'google/chrome/extension',
               'google/chromeos/device',
               'google/chromeos/publicaccount',
               'google/chromeos/user',
               'google/chrome/user',
               'google/ios/user')):
-        if request_type != 'policy':
-          fetch_response.error_code = 400
-          fetch_response.error_message = 'Invalid request type'
-        else:
-          self.ProcessCloudPolicy(request, token_info, fetch_response)
+        fetch_response = response.policy_response.response.add()
+        self.ProcessCloudPolicy(request, token_info, fetch_response)
+      elif request.policy_type == 'google/chrome/extension':
+        self.ProcessCloudPolicyForExtensions(
+            request, response.policy_response, token_info)
       else:
         fetch_response.error_code = 400
         fetch_response.error_message = 'Invalid policy_type'
@@ -532,14 +530,14 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     return (200, response)
 
   def SetProtobufMessageField(self, group_message, field, field_value):
-    '''Sets a field in a protobuf message.
+    """Sets a field in a protobuf message.
 
     Args:
       group_message: The protobuf message.
       field: The field of the message to set, it should be a member of
           group_message.DESCRIPTOR.fields.
       field_value: The value to set.
-    '''
+    """
     if field.label == field.LABEL_REPEATED:
       assert type(field_value) == list
       entries = group_message.__getattribute__(field.name)
@@ -577,13 +575,13 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     group_message.__setattr__(field.name, field_value)
 
   def GatherDevicePolicySettings(self, settings, policies):
-    '''Copies all the policies from a dictionary into a protobuf of type
+    """Copies all the policies from a dictionary into a protobuf of type
     CloudDeviceSettingsProto.
 
     Args:
       settings: The destination ChromeDeviceSettingsProto protobuf.
       policies: The source dictionary containing policies in JSON format.
-    '''
+    """
     for group in settings.DESCRIPTOR.fields:
       # Create protobuf message for group.
       group_message = eval('dp.' + group.message_type.name + '()')
@@ -601,14 +599,14 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
         settings.__getattribute__(group.name).CopyFrom(group_message)
 
   def GatherUserPolicySettings(self, settings, policies):
-    '''Copies all the policies from a dictionary into a protobuf of type
+    """Copies all the policies from a dictionary into a protobuf of type
     CloudPolicySettings.
 
     Args:
       settings: The destination: a CloudPolicySettings protobuf.
       policies: The source: a dictionary containing policies under keys
           'recommended' and 'mandatory'.
-    '''
+    """
     for field in settings.DESCRIPTOR.fields:
       # |field| is the entry for a specific policy in the top-level
       # CloudPolicySettings proto.
@@ -630,6 +628,33 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
       self.SetProtobufMessageField(policy_message, field_descriptor, value)
       settings.__getattribute__(field.name).CopyFrom(policy_message)
 
+  def ProcessCloudPolicyForExtensions(self, request, response, token_info):
+    """Handles a request for policy for extensions.
+
+    A request for policy for extensions is slightly different from the other
+    cloud policy requests, because it can trigger 0, one or many
+    PolicyFetchResponse messages in the response.
+
+    Args:
+      request: The PolicyFetchRequest that triggered this handler.
+      response: The DevicePolicyResponse message for the response. Multiple
+      PolicyFetchResponses will be appended to this message.
+      token_info: The token extracted from the request.
+    """
+    # Send one PolicyFetchResponse for each extension that has
+    # configuration data at the server.
+    ids = self.server.ListMatchingComponents('google/chrome/extension')
+    for settings_entity_id in ids:
+      # Reuse the extension policy request, to trigger the same signature
+      # type in the response.
+      request.settings_entity_id = settings_entity_id
+      fetch_response = response.response.add()
+      self.ProcessCloudPolicy(request, token_info, fetch_response)
+      # Don't do key rotations for these messages.
+      fetch_response.ClearField('new_public_key')
+      fetch_response.ClearField('new_public_key_signature')
+      fetch_response.ClearField('new_public_key_verification_signature')
+
   def ProcessCloudPolicy(self, msg, token_info, response):
     """Handles a cloud policy request. (New protocol for policy requests.)
 
@@ -638,7 +663,7 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
 
     Args:
       msg: The CloudPolicyRequest message received from the client.
-      token_info: the token extracted from the request.
+      token_info: The token extracted from the request.
       response: A PolicyFetchResponse message that should be filled with the
                 response data.
     """
@@ -1046,7 +1071,7 @@ class PolicyTestServer(testserver_base.BrokenPipeHandlerMixIn,
     """Returns the base filename for the given policy_selector.
 
     Args:
-      policy_selector: the policy type and settings entity id, joined by '/'.
+      policy_selector: The policy type and settings entity id, joined by '/'.
 
     Returns:
       The filename corresponding to the policy_selector, without a file
@@ -1056,6 +1081,22 @@ class PolicyTestServer(testserver_base.BrokenPipeHandlerMixIn,
     return os.path.join(self.data_dir or '',
                         'policy_%s' % sanitized_policy_selector)
 
+  def ListMatchingComponents(self, policy_type):
+    """Returns a list of settings entity IDs that have a configuration file.
+
+    Args:
+      policy_type: The policy type to look for. Only settings entity IDs for
+      file selectors That match this policy_type will be returned.
+
+    Returns:
+      A list of settings entity IDs for the given |policy_type| that have a
+      configuration file in this server (either as a .bin, .txt or .data file).
+    """
+    base_name = self.GetBaseFilename(policy_type)
+    files = glob.glob('%s_*.*' % base_name)
+    len_base_name = len(base_name) + 1
+    return [ file[len_base_name:file.rfind('.')] for file in files ]
+
   def ReadPolicyFromDataDir(self, policy_selector, proto_message):
     """Tries to read policy payload from a file in the data directory.
 

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

@@ -35,13 +35,6 @@ bool NotInSchemaMap(const scoped_refptr<SchemaMap> schema_map,
   return schema_map->GetSchema(PolicyNamespace(domain, component_id)) == NULL;
 }
 
-bool ToPolicyNamespaceKey(const PolicyNamespace& ns, PolicyNamespaceKey* key) {
-  if (!ComponentCloudPolicyStore::GetPolicyType(ns.domain, &key->first))
-    return false;
-  key->second = ns.component_id;
-  return true;
-}
-
 bool ToPolicyNamespace(const PolicyNamespaceKey& key, PolicyNamespace* ns) {
   if (!ComponentCloudPolicyStore::GetPolicyDomain(key.first, &ns->domain))
     return false;
@@ -293,7 +286,7 @@ void ComponentCloudPolicyService::OnSchemaRegistryUpdated(
   if (!loaded_initial_policy_)
     return;
 
-  SetCurrentSchema();
+  ReloadSchema();
 }
 
 void ComponentCloudPolicyService::OnCoreConnected(CloudPolicyCore* core) {
@@ -302,13 +295,13 @@ void ComponentCloudPolicyService::OnCoreConnected(CloudPolicyCore* core) {
 
   core_->client()->AddObserver(this);
 
+  // Register the supported policy domains at the client.
+  core_->client()->AddNamespaceToFetch(
+      PolicyNamespaceKey(dm_protocol::kChromeExtensionPolicyType, ""));
+
   // Immediately load any PolicyFetchResponses that the client may already
   // have.
   OnPolicyFetched(core_->client());
-
-  // Register the current namespaces at the client.
-  current_schema_map_ = new SchemaMap();
-  SetCurrentSchema();
 }
 
 void ComponentCloudPolicyService::OnCoreDisconnecting(CloudPolicyCore* core) {
@@ -318,15 +311,8 @@ void ComponentCloudPolicyService::OnCoreDisconnecting(CloudPolicyCore* core) {
   core_->client()->RemoveObserver(this);
 
   // Remove all the namespaces from the client.
-  scoped_refptr<SchemaMap> empty = new SchemaMap();
-  PolicyNamespaceList removed;
-  PolicyNamespaceList added;
-  empty->GetChanges(current_schema_map_, &removed, &added);
-  for (size_t i = 0; i < removed.size(); ++i) {
-    PolicyNamespaceKey key;
-    if (ToPolicyNamespaceKey(removed[i], &key))
-      core_->client()->RemoveNamespaceToFetch(key);
-  }
+  core_->client()->RemoveNamespaceToFetch(
+      PolicyNamespaceKey(dm_protocol::kChromeExtensionPolicyType, ""));
 }
 
 void ComponentCloudPolicyService::OnRefreshSchedulerStarted(
@@ -454,20 +440,18 @@ void ComponentCloudPolicyService::OnBackendInitialized(
   // We're now ready to serve the initial policy; notify the policy observers.
   OnPolicyUpdated(initial_policy.Pass());
 
+  // Send the current schema to the backend, in case it has changed while the
+  // backend was initializing.
+  ReloadSchema();
+
   // Start observing the core and tracking the state of the client.
   core_->AddObserver(this);
 
-  if (core_->client()) {
+  if (core_->client())
     OnCoreConnected(core_);
-  } else {
-    // Send the current schema to the backend, in case it has changed while the
-    // backend was initializing. OnCoreConnected() also does this if a client is
-    // already connected.
-    SetCurrentSchema();
-  }
 }
 
-void ComponentCloudPolicyService::SetCurrentSchema() {
+void ComponentCloudPolicyService::ReloadSchema() {
   DCHECK(CalledOnValidThread());
 
   scoped_ptr<PolicyNamespaceList> removed(new PolicyNamespaceList);
@@ -478,26 +462,12 @@ void ComponentCloudPolicyService::SetCurrentSchema() {
 
   current_schema_map_ = new_schema_map;
 
-  if (core_->client()) {
-    for (size_t i = 0; i < removed->size(); ++i) {
-      PolicyNamespaceKey key;
-      if (ToPolicyNamespaceKey((*removed)[i], &key))
-        core_->client()->RemoveNamespaceToFetch(key);
-    }
-
-    bool added_namespaces_to_client = false;
-    for (size_t i = 0; i < added.size(); ++i) {
-      PolicyNamespaceKey key;
-      if (ToPolicyNamespaceKey(added[i], &key)) {
-        core_->client()->AddNamespaceToFetch(key);
-        added_namespaces_to_client = true;
-      }
-    }
-
-    if (added_namespaces_to_client)
-      core_->RefreshSoon();
-  }
+  // Schedule a policy refresh if a new managed component was added.
+  if (core_->client() && !added.empty())
+    core_->RefreshSoon();
 
+  // Send the updated SchemaMap and a list of removed components to the
+  // backend.
   backend_task_runner_->PostTask(FROM_HERE,
                                  base::Bind(&Backend::OnSchemasUpdated,
                                             base::Unretained(backend_.get()),

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

@@ -121,7 +121,7 @@ class POLICY_EXPORT ComponentCloudPolicyService
 
   void InitializeIfReady();
   void OnBackendInitialized(scoped_ptr<PolicyBundle> initial_policy);
-  void SetCurrentSchema();
+  void ReloadSchema();
   void OnPolicyUpdated(scoped_ptr<PolicyBundle> policy);
 
   Delegate* delegate_;

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

@@ -165,11 +165,7 @@ class ComponentCloudPolicyServiceTest : public testing::Test {
 
     // Also initialize the refresh scheduler, so that calls to
     // core()->RefreshSoon() trigger a FetchPolicy() call on the mock |client_|.
-    // Expect the initial refresh now, if the store doesn't have policy (if it
-    // does then the CloudPolicyRefreshScheduler won't start refreshing until
-    // invalidations are available, or a timeout elapses).
-    if (!store_.has_policy())
-      EXPECT_CALL(*client_, FetchPolicy());
+    EXPECT_CALL(*client_, FetchPolicy());
     core_.StartRefreshScheduler();
     RunUntilIdle();
     Mock::VerifyAndClearExpectations(client_);
@@ -492,7 +488,7 @@ TEST_F(ComponentCloudPolicyServiceTest, SignInAfterStartup) {
 }
 
 TEST_F(ComponentCloudPolicyServiceTest, SignOut) {
-  // Initialize everthing and serve policy for a component.
+  // Initialize everything and serve policy for a component.
   PopulateCache();
   LoadStore();
   InitializeRegistry();

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

@@ -19,25 +19,27 @@ namespace {
 
 // Verifies that |value| is not empty and encodes it into base64url format,
 // which is safe to use as a file name on all platforms.
-bool Base64Encode(const std::string& value, std::string* encoded) {
+bool Base64UrlEncode(const std::string& value, std::string* encoded) {
   DCHECK(!value.empty());
   if (value.empty())
     return false;
   base::Base64Encode(value, encoded);
   base::ReplaceChars(*encoded, "+", "-", encoded);
   base::ReplaceChars(*encoded, "/", "_", encoded);
+  // Note: this encoding keeps the padding chars, though the "Baset64 with safe
+  // URL alphabet" encoding trims them. See Base64UrlDecode below.
   return true;
 }
 
 // Decodes all elements of |input| from base64url format and stores the decoded
 // elements in |output|.
-bool Base64Encode(const std::set<std::string>& input,
-                  std::set<std::string>* output) {
+bool Base64UrlEncode(const std::set<std::string>& input,
+                     std::set<std::string>* output) {
   output->clear();
   for (std::set<std::string>::const_iterator it = input.begin();
        it != input.end(); ++it) {
     std::string encoded;
-    if (!Base64Encode(*it, &encoded)) {
+    if (!Base64UrlEncode(*it, &encoded)) {
       output->clear();
       return false;
     }
@@ -48,7 +50,7 @@ bool Base64Encode(const std::set<std::string>& input,
 
 // Decodes |encoded| from base64url format and verifies that the result is not
 // emtpy.
-bool Base64Decode(const std::string& encoded, std::string* value) {
+bool Base64UrlDecode(const std::string& encoded, std::string* value) {
   std::string buffer;
   base::ReplaceChars(encoded, "-", "+", &buffer);
   base::ReplaceChars(buffer, "_", "/", &buffer);
@@ -120,7 +122,7 @@ void ResourceCache::LoadAllSubkeys(
     // Only read from |subkey_path| if it is not a symlink and its name is
     // a base64-encoded string.
     if (!base::IsLink(path) &&
-        Base64Decode(encoded_subkey, &subkey) &&
+        Base64UrlDecode(encoded_subkey, &subkey) &&
         base::ReadFileToString(path, &data)) {
       (*contents)[subkey].swap(data);
     }
@@ -159,7 +161,7 @@ void ResourceCache::FilterSubkeys(const std::string& key,
     std::string subkey;
     // Delete files with invalid names, and files whose subkey doesn't pass the
     // filter.
-    if (!Base64Decode(subkey_path.BaseName().MaybeAsASCII(), &subkey) ||
+    if (!Base64UrlDecode(subkey_path.BaseName().MaybeAsASCII(), &subkey) ||
         test.Run(subkey)) {
       base::DeleteFile(subkey_path, true);
     }
@@ -174,7 +176,7 @@ void ResourceCache::FilterSubkeys(const std::string& key,
 void ResourceCache::PurgeOtherKeys(const std::set<std::string>& keys_to_keep) {
   DCHECK(task_runner_->RunsTasksOnCurrentThread());
   std::set<std::string> encoded_keys_to_keep;
-  if (!Base64Encode(keys_to_keep, &encoded_keys_to_keep))
+  if (!Base64UrlEncode(keys_to_keep, &encoded_keys_to_keep))
     return;
 
   base::FileEnumerator enumerator(
@@ -196,7 +198,7 @@ void ResourceCache::PurgeOtherSubkeys(
     return;
 
   std::set<std::string> encoded_subkeys_to_keep;
-  if (!Base64Encode(subkeys_to_keep, &encoded_subkeys_to_keep))
+  if (!Base64UrlEncode(subkeys_to_keep, &encoded_subkeys_to_keep))
     return;
 
   base::FileEnumerator enumerator(key_path, false, base::FileEnumerator::FILES);
@@ -216,7 +218,7 @@ bool ResourceCache::VerifyKeyPath(const std::string& key,
                                   bool allow_create,
                                   base::FilePath* path) {
   std::string encoded;
-  if (!Base64Encode(key, &encoded))
+  if (!Base64UrlEncode(key, &encoded))
     return false;
   *path = cache_dir_.AppendASCII(encoded);
   return allow_create ? base::CreateDirectory(*path) :
@@ -230,7 +232,7 @@ bool ResourceCache::VerifyKeyPathAndGetSubkeyPath(const std::string& key,
   base::FilePath key_path;
   std::string encoded;
   if (!VerifyKeyPath(key, allow_create_key, &key_path) ||
-      !Base64Encode(subkey, &encoded)) {
+      !Base64UrlEncode(subkey, &encoded)) {
     return false;
   }
   *path = key_path.AppendASCII(encoded);