Fix for "chrome://" links in PDFs.

content/browser/child_process_security_policy_impl.cc

Index: content/browser/child_process_security_policy_impl.cc
diff --git a/content/browser/child_process_security_policy_impl.cc b/content/browser/child_process_security_policy_impl.cc
index b748a72fe73236016d23c5a514b2b828f8b26779..55e6bad546ee67ee0ac424f3c8d6218098c96674 100644
--- a/content/browser/child_process_security_policy_impl.cc
+++ b/content/browser/child_process_security_policy_impl.cc
@@ -4,6 +4,8 @@
 
 #include "content/browser/child_process_security_policy_impl.h"
 
+#include <utility>
+
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
@@ -92,11 +94,21 @@ class ChildProcessSecurityPolicyImpl::SecurityState {
     scheme_policy_[scheme] = true;
   }
 
+  // Grant permission to request URLs with both the specified scheme and host.
+  void GrantSchemeHost(const std::string& scheme, const std::string& host) {
+    scheme_host_policy_[std::make_pair(scheme, host)] = true;
+  }
+
   // Revoke permission to request URLs with the specified scheme.
   void RevokeScheme(const std::string& scheme) {
     scheme_policy_[scheme] = false;
   }
 
+  // Revoke permission to request URLs with both the specified scheme and host.
+  void RevokeSchemeHost(const std::string& scheme, const std::string& host) {

[Charlie Reis, 2015/09/22 17:38:15]

We shouldn't introduce new methods until they're needed.

[paulmeyer, 2015/09/22 22:13:57]

I was thinking the same thing, though RevokeScheme is exactly the same situation
(it is not currently used). Should I also remove RevokeScheme?

[Charlie Reis, 2015/09/23 00:07:15]

Not in this CL.  We can probably remove it as dead code separately.

[paulmeyer, 2015/09/23 17:03:53]

Acknowledged.

+    scheme_host_policy_[std::make_pair(scheme, host)] = false;
+  }
+
   // Grant certain permissions to a file.
   void GrantPermissionsForFile(const base::FilePath& file, int permissions) {
     base::FilePath stripped = file.StripTrailingSeparators();
@@ -168,10 +180,18 @@ class ChildProcessSecurityPolicyImpl::SecurityState {
 
   // Determine whether permission has been granted to commit |url|.
   bool CanCommitURL(const GURL& url) {
-    // Having permission to a scheme implies permssion to all of its URLs.
-    SchemeMap::const_iterator judgment(scheme_policy_.find(url.scheme()));
-    if (judgment != scheme_policy_.end())
-      return judgment->second;
+    // Check for permission for specific scheme and host.
+    SchemeHostMap::const_iterator scheme_host_judgment(
+        scheme_host_policy_.find(std::make_pair(url.scheme(), url.host())));
+    if (scheme_host_judgment != scheme_host_policy_.end())
+      return scheme_host_judgment->second;
+
+    // Otherwise, having permission to a scheme implies permission to all of its
+    // URLs.
+    SchemeMap::const_iterator scheme_judgment(
+        scheme_policy_.find(url.scheme()));
+    if (scheme_judgment != scheme_policy_.end())
+      return scheme_judgment->second;
 
     // file:// URLs are more granular.  The child may have been given
     // permission to a specific file but not the file:// scheme in general.
@@ -242,6 +262,7 @@ class ChildProcessSecurityPolicyImpl::SecurityState {
 
  private:
   typedef std::map<std::string, bool> SchemeMap;
+  typedef std::map<std::pair<std::string, std::string>, bool> SchemeHostMap;
 
   typedef int FilePermissionFlags;  // bit-set of base::File::Flags
   typedef std::map<base::FilePath, FilePermissionFlags> FileMap;
@@ -255,6 +276,20 @@ class ChildProcessSecurityPolicyImpl::SecurityState {
   // or revoked.
   SchemeMap scheme_policy_;
 
+  // Maps URL (scheme, host) pairs to whether permission has been granted or
+  // revoked:
+  //   |true| means the (scheme, host) pair has been granted.
+  //   |false| means the (scheme, host) pair has been revoked.
+  // If a (scheme, host) pair is not present in the map, then it has never been
+  // granted or revoked.

[Charlie Reis, 2015/09/22 17:38:15]

This seems overly complicated if we don't have any code that does revocation.

It seems easier to just have a set of GURLs or Origins (e.g., chrome://favicon),
similar to FileSet.

[paulmeyer, 2015/09/22 22:13:57]

Okay, I'll use a set of origins.

+  //
+  // For schemes that are present in both |scheme_policy_| and
+  // |scheme_host_policy_|, the permission set for specific hosts within a
+  // scheme in |scheme_host_polcy_| will be respected first, followed by the

[Charlie Reis, 2015/09/22 17:38:15]

This also seems overly complicated.  If we aren't doing revocation, then there's
no reason to check the schemes+hosts before checking schemes.  If a full scheme
is granted, that should imply that all hosts in it are granted.

[paulmeyer, 2015/09/22 22:13:57]

Done.

+  // general permission for the scheme in |scheme_policy_| for all other hosts
+  // within that scheme.
+  SchemeHostMap scheme_host_policy_;
+
   // The set of files the child process is permited to upload to the web.
   FileMap file_permissions_;
 
@@ -514,6 +549,18 @@ void ChildProcessSecurityPolicyImpl::GrantScheme(int child_id,
   state->second->GrantScheme(scheme);
 }
 
+void ChildProcessSecurityPolicyImpl::GrantSchemeHost(int child_id,
+                                                     const std::string& scheme,
+                                                     const std::string& host) {
+  base::AutoLock lock(lock_);
+
+  SecurityStateMap::iterator state = security_state_.find(child_id);
+  if (state == security_state_.end())
+    return;
+
+  state->second->GrantSchemeHost(scheme, host);
+}
+
 void ChildProcessSecurityPolicyImpl::GrantWebUIBindings(int child_id) {
   base::AutoLock lock(lock_);