ChildProcessSecurityPolicy: Remove CanWriteFile and CanCreateFile.

We currently have some checks that are more fine-grained than the grants. There's no reason for this, so I'm removing them and replacing them with the broader check.

I would also like to remove some of the Can*FileSystemFile checks, but I currently cannot, as that goes into checking the permissions for the filesystem as a whole in the case of isolated file systems.

That will have to be saved for a different CL.

This CL depends on:
https://codereview.chromium.org/24406003/

This is part of Step 2 of this refactoring plan:
https://docs.google.com/a/google.com/document/d/1QGkGWuwgSuaRqovz4wyb0upqPKDVsgYOFKt44E7gmOE/edit?usp=sharing

BUG=262142, 263150

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

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

7 files changed, 15 insertions, 60 deletions

diff --git a/content/browser/child_process_security_policy_impl.cc b/content/browser/child_process_security_policy_impl.cc
index b0a2b6e..c6f9757 100644
--- a/content/browser/child_process_security_policy_impl.cc
+++ b/content/browser/child_process_security_policy_impl.cc
@@ -614,16 +614,6 @@ bool ChildProcessSecurityPolicyImpl::CanReadFile(int child_id,
   return HasPermissionsForFile(child_id, file, kReadFilePermissions);
 }
 
-bool ChildProcessSecurityPolicyImpl::CanWriteFile(int child_id,
-                                                  const base::FilePath& file) {
-  return HasPermissionsForFile(child_id, file, kWriteFilePermissions);
-}
-
-bool ChildProcessSecurityPolicyImpl::CanCreateFile(int child_id,
-                                                   const base::FilePath& file) {
-  return HasPermissionsForFile(child_id, file, kCreateFilePermissions);
-}
-
 bool ChildProcessSecurityPolicyImpl::CanCreateReadWriteFile(
     int child_id,
     const base::FilePath& file) {
diff --git a/content/browser/child_process_security_policy_impl.h b/content/browser/child_process_security_policy_impl.h
index 0396b54..b1f68e4 100644
--- a/content/browser/child_process_security_policy_impl.h
+++ b/content/browser/child_process_security_policy_impl.h
@@ -59,8 +59,6 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
       const std::string& filesystem_id) OVERRIDE;
   virtual void GrantScheme(int child_id, const std::string& scheme) OVERRIDE;
   virtual bool CanReadFile(int child_id, const base::FilePath& file) OVERRIDE;
-  virtual bool CanWriteFile(int child_id, const base::FilePath& file) OVERRIDE;
-  virtual bool CanCreateFile(int child_id, const base::FilePath& file) OVERRIDE;
   virtual bool CanCreateReadWriteFile(int child_id,
                                       const base::FilePath& file) OVERRIDE;
   virtual bool CanReadFileSystem(int child_id,
diff --git a/content/browser/child_process_security_policy_unittest.cc b/content/browser/child_process_security_policy_unittest.cc
index e7cb88b..9f574c5 100644
--- a/content/browser/child_process_security_policy_unittest.cc
+++ b/content/browser/child_process_security_policy_unittest.cc
@@ -382,8 +382,6 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
 
   // Test initially having no permissions.
   EXPECT_FALSE(p->CanReadFile(kRendererID, file));
-  EXPECT_FALSE(p->CanWriteFile(kRendererID, file));
-  EXPECT_FALSE(p->CanCreateFile(kRendererID, file));
   EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
   EXPECT_FALSE(p->CanReadFileSystemFile(kRendererID, url));
   EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
@@ -393,8 +391,6 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
   // Testing every combination of permissions granting and revoking.
   p->GrantReadFile(kRendererID, file);
   EXPECT_TRUE(p->CanReadFile(kRendererID, file));
-  EXPECT_FALSE(p->CanWriteFile(kRendererID, file));
-  EXPECT_FALSE(p->CanCreateFile(kRendererID, file));
   EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
   EXPECT_TRUE(p->CanReadFileSystemFile(kRendererID, url));
   EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
@@ -402,8 +398,6 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
   EXPECT_FALSE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
   p->RevokeAllPermissionsForFile(kRendererID, file);
   EXPECT_FALSE(p->CanReadFile(kRendererID, file));
-  EXPECT_FALSE(p->CanWriteFile(kRendererID, file));
-  EXPECT_FALSE(p->CanCreateFile(kRendererID, file));
   EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
   EXPECT_FALSE(p->CanReadFileSystemFile(kRendererID, url));
   EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
@@ -412,8 +406,6 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
 
   p->GrantCreateReadWriteFile(kRendererID, file);
   EXPECT_TRUE(p->CanReadFile(kRendererID, file));
-  EXPECT_TRUE(p->CanWriteFile(kRendererID, file));
-  EXPECT_TRUE(p->CanCreateFile(kRendererID, file));
   EXPECT_TRUE(p->CanCreateReadWriteFile(kRendererID, file));
   EXPECT_TRUE(p->CanReadFileSystemFile(kRendererID, url));
   EXPECT_TRUE(p->CanWriteFileSystemFile(kRendererID, url));
@@ -421,8 +413,6 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
   EXPECT_TRUE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
   p->RevokeAllPermissionsForFile(kRendererID, file);
   EXPECT_FALSE(p->CanReadFile(kRendererID, file));
-  EXPECT_FALSE(p->CanWriteFile(kRendererID, file));
-  EXPECT_FALSE(p->CanCreateFile(kRendererID, file));
   EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
   EXPECT_FALSE(p->CanReadFileSystemFile(kRendererID, url));
   EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
@@ -432,8 +422,6 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
   // Test revoke permissions on renderer ID removal.
   p->GrantCreateReadWriteFile(kRendererID, file);
   EXPECT_TRUE(p->CanReadFile(kRendererID, file));
-  EXPECT_TRUE(p->CanWriteFile(kRendererID, file));
-  EXPECT_TRUE(p->CanCreateFile(kRendererID, file));
   EXPECT_TRUE(p->CanCreateReadWriteFile(kRendererID, file));
   EXPECT_TRUE(p->CanReadFileSystemFile(kRendererID, url));
   EXPECT_TRUE(p->CanWriteFileSystemFile(kRendererID, url));
@@ -441,8 +429,6 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
   EXPECT_TRUE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
   p->Remove(kRendererID);
   EXPECT_FALSE(p->CanReadFile(kRendererID, file));
-  EXPECT_FALSE(p->CanWriteFile(kRendererID, file));
-  EXPECT_FALSE(p->CanCreateFile(kRendererID, file));
   EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
   EXPECT_FALSE(p->CanReadFileSystemFile(kRendererID, url));
   EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
@@ -452,8 +438,6 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
   // Test having no permissions upon re-adding same renderer ID.
   p->Add(kRendererID);
   EXPECT_FALSE(p->CanReadFile(kRendererID, file));
-  EXPECT_FALSE(p->CanWriteFile(kRendererID, file));
-  EXPECT_FALSE(p->CanCreateFile(kRendererID, file));
   EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
   EXPECT_FALSE(p->CanReadFileSystemFile(kRendererID, url));
   EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
diff --git a/content/browser/renderer_host/pepper/pepper_external_file_ref_backend.cc b/content/browser/renderer_host/pepper/pepper_external_file_ref_backend.cc
index 096b908..296a422 100644
--- a/content/browser/renderer_host/pepper/pepper_external_file_ref_backend.cc
+++ b/content/browser/renderer_host/pepper/pepper_external_file_ref_backend.cc
@@ -109,26 +109,18 @@ int32_t PepperExternalFileRefBackend::CanRead() const {
 }
 
 int32_t PepperExternalFileRefBackend::CanWrite() const {
-  if (!ChildProcessSecurityPolicyImpl::GetInstance()->
-          CanWriteFile(render_process_id_, path_)) {
-    return PP_ERROR_NOACCESS;
-  }
-  return PP_OK;
+  // Platform files have coarse-grained grants in ChildProcessSecurityPolicy.
+  return CanReadWrite();
 }
 
 int32_t PepperExternalFileRefBackend::CanCreate() const {
-  if (!ChildProcessSecurityPolicyImpl::GetInstance()->
-          CanCreateFile(render_process_id_, path_)) {
-    return PP_ERROR_NOACCESS;
-  }
-  return PP_OK;
+  // Platform files have coarse-grained grants in ChildProcessSecurityPolicy.
+  return CanReadWrite();
 }
 
 int32_t PepperExternalFileRefBackend::CanReadWrite() const {
-  ChildProcessSecurityPolicyImpl* policy =
-        ChildProcessSecurityPolicyImpl::GetInstance();
-  if (!policy->CanReadFile(render_process_id_, path_) ||
-      !policy->CanWriteFile(render_process_id_, path_)) {
+  if (!ChildProcessSecurityPolicyImpl::GetInstance()->
+          CanCreateReadWriteFile(render_process_id_, path_)) {
     return PP_ERROR_NOACCESS;
   }
   return PP_OK;
diff --git a/content/browser/renderer_host/pepper/pepper_flash_file_message_filter.cc b/content/browser/renderer_host/pepper/pepper_flash_file_message_filter.cc
index ef74062..7e010d6 100644
--- a/content/browser/renderer_host/pepper/pepper_flash_file_message_filter.cc
+++ b/content/browser/renderer_host/pepper/pepper_flash_file_message_filter.cc
@@ -31,16 +31,9 @@ bool CanRead(int process_id, const base::FilePath& path) {
       CanReadFile(process_id, path);
 }
 
-bool CanWrite(int process_id, const base::FilePath& path) {
+bool CanCreateReadWrite(int process_id, const base::FilePath& path) {
   return ChildProcessSecurityPolicyImpl::GetInstance()->
-      CanWriteFile(process_id, path);
-}
-
-bool CanReadWrite(int process_id, const base::FilePath& path) {
-  ChildProcessSecurityPolicyImpl* policy =
-      ChildProcessSecurityPolicyImpl::GetInstance();
-  return policy->CanReadFile(process_id, path) &&
-      policy->CanWriteFile(process_id, path);
+      CanCreateReadWriteFile(process_id, path);
 }
 
 }  // namespace
@@ -163,9 +156,9 @@ int32_t PepperFlashFileMessageFilter::OnRenameFile(
     const ppapi::PepperFilePath& from_path,
     const ppapi::PepperFilePath& to_path) {
   base::FilePath from_full_path = ValidateAndConvertPepperFilePath(
-      from_path, base::Bind(&CanWrite));
+      from_path, base::Bind(&CanCreateReadWrite));
   base::FilePath to_full_path = ValidateAndConvertPepperFilePath(
-      to_path, base::Bind(&CanWrite));
+      to_path, base::Bind(&CanCreateReadWrite));
   if (from_full_path.empty() || to_full_path.empty()) {
     return ppapi::PlatformFileErrorToPepperError(
         base::PLATFORM_FILE_ERROR_ACCESS_DENIED);
@@ -181,7 +174,7 @@ int32_t PepperFlashFileMessageFilter::OnDeleteFileOrDir(
     const ppapi::PepperFilePath& path,
     bool recursive) {
   base::FilePath full_path = ValidateAndConvertPepperFilePath(
-      path, base::Bind(&CanWrite));
+      path, base::Bind(&CanCreateReadWrite));
   if (full_path.empty()) {
     return ppapi::PlatformFileErrorToPepperError(
         base::PLATFORM_FILE_ERROR_ACCESS_DENIED);
@@ -195,7 +188,7 @@ int32_t PepperFlashFileMessageFilter::OnCreateDir(
     ppapi::host::HostMessageContext* context,
     const ppapi::PepperFilePath& path) {
   base::FilePath full_path = ValidateAndConvertPepperFilePath(
-      path, base::Bind(&CanWrite));
+      path, base::Bind(&CanCreateReadWrite));
   if (full_path.empty()) {
     return ppapi::PlatformFileErrorToPepperError(
         base::PLATFORM_FILE_ERROR_ACCESS_DENIED);
@@ -257,7 +250,7 @@ int32_t PepperFlashFileMessageFilter::OnCreateTemporaryFile(
   ppapi::PepperFilePath dir_path(
       ppapi::PepperFilePath::DOMAIN_MODULE_LOCAL, base::FilePath());
   base::FilePath validated_dir_path = ValidateAndConvertPepperFilePath(
-      dir_path, base::Bind(&CanReadWrite));
+      dir_path, base::Bind(&CanCreateReadWrite));
   if (validated_dir_path.empty() ||
       (!base::DirectoryExists(validated_dir_path) &&
        !file_util::CreateDirectory(validated_dir_path))) {
diff --git a/content/browser/renderer_host/pepper/pepper_security_helper.cc b/content/browser/renderer_host/pepper/pepper_security_helper.cc
index a07f5dc..4e1306e 100644
--- a/content/browser/renderer_host/pepper/pepper_security_helper.cc
+++ b/content/browser/renderer_host/pepper/pepper_security_helper.cc
@@ -65,8 +65,8 @@ bool CanOpenWithPepperFlags(int pp_open_flags, int child_id,
                             const base::FilePath& file) {
   return CanOpenFileWithPepperFlags(
       &ChildProcessSecurityPolicyImpl::CanReadFile,
-      &ChildProcessSecurityPolicyImpl::CanWriteFile,
-      &ChildProcessSecurityPolicyImpl::CanCreateFile,
+      &ChildProcessSecurityPolicyImpl::CanCreateReadWriteFile,
+      &ChildProcessSecurityPolicyImpl::CanCreateReadWriteFile,
       &ChildProcessSecurityPolicyImpl::CanCreateReadWriteFile,
       pp_open_flags, child_id, file);
 }
diff --git a/content/public/browser/child_process_security_policy.h b/content/public/browser/child_process_security_policy.h
index f42f45d..22a3d7e 100644
--- a/content/public/browser/child_process_security_policy.h
+++ b/content/public/browser/child_process_security_policy.h
@@ -58,8 +58,6 @@ class ChildProcessSecurityPolicy {
   // browser should call this method to determine whether the process has the
   // capability to upload the requested file.
   virtual bool CanReadFile(int child_id, const base::FilePath& file) = 0;
-  virtual bool CanWriteFile(int child_id, const base::FilePath& file) = 0;
-  virtual bool CanCreateFile(int child_id, const base::FilePath& file) = 0;
   virtual bool CanCreateReadWriteFile(int child_id,
                                       const base::FilePath& file) = 0;