ChildProcessSecurityPolicy: Add DeleteFromFileSystem permission.

BUG=275980

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

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

7 files changed, 102 insertions, 67 deletions

diff --git a/chrome/browser/devtools/devtools_file_helper.cc b/chrome/browser/devtools/devtools_file_helper.cc
index 38e49bc..d8372a4 100644
--- a/chrome/browser/devtools/devtools_file_helper.cc
+++ b/chrome/browser/devtools/devtools_file_helper.cc
@@ -152,6 +152,7 @@ std::string RegisterFileSystem(WebContents* web_contents,
   policy->GrantReadFileSystem(renderer_id, file_system_id);
   policy->GrantWriteFileSystem(renderer_id, file_system_id);
   policy->GrantCreateFileForFileSystem(renderer_id, file_system_id);
+  policy->GrantDeleteFromFileSystem(renderer_id, file_system_id);
 
   // We only need file level access for reading FileEntries. Saving FileEntries
   // just needs the file system to have read/write access, which is granted
diff --git a/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc b/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc
index 81648f0..ca64e69 100644
--- a/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc
+++ b/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc
@@ -313,6 +313,7 @@ GrantedFileEntry CreateFileEntry(
   policy->GrantReadFileSystem(renderer_id, result.filesystem_id);
   if (HasFileSystemWritePermission(extension)) {
     policy->GrantWriteFileSystem(renderer_id, result.filesystem_id);
+    policy->GrantDeleteFromFileSystem(renderer_id, result.filesystem_id);
     if (is_directory)
       policy->GrantCreateFileForFileSystem(renderer_id, result.filesystem_id);
   }
diff --git a/content/browser/child_process_security_policy_impl.cc b/content/browser/child_process_security_policy_impl.cc
index e91367f..5e069c4 100644
--- a/content/browser/child_process_security_policy_impl.cc
+++ b/content/browser/child_process_security_policy_impl.cc
@@ -38,9 +38,10 @@ enum ChildProcessSecurityPermissions {
   WRITE_FILE_PERMISSION            = 1 << 1,
   CREATE_NEW_FILE_PERMISSION       = 1 << 2,
   CREATE_OVERWRITE_FILE_PERMISSION = 1 << 3,
+  DELETE_FILE_PERMISSION           = 1 << 4,
 
   // Used by Media Galleries API
-  COPY_INTO_FILE_PERMISSION        = 1 << 4,
+  COPY_INTO_FILE_PERMISSION        = 1 << 5,
 };
 
 // Used internally only. Bitmasks that are actually used by the Grant* and Can*
@@ -56,9 +57,11 @@ enum ChildProcessSecurityGrants {
                                  CREATE_OVERWRITE_FILE_PERMISSION |
                                  READ_FILE_PERMISSION |
                                  WRITE_FILE_PERMISSION |
-                                 COPY_INTO_FILE_PERMISSION,
+                                 COPY_INTO_FILE_PERMISSION |
+                                 DELETE_FILE_PERMISSION,
 
   COPY_INTO_FILE_GRANT         = COPY_INTO_FILE_PERMISSION,
+  DELETE_FILE_GRANT            = DELETE_FILE_PERMISSION,
 };
 
 }  // namespace
@@ -478,6 +481,11 @@ void ChildProcessSecurityPolicyImpl::GrantCopyIntoFileSystem(
   GrantPermissionsForFileSystem(child_id, filesystem_id, COPY_INTO_FILE_GRANT);
 }
 
+void ChildProcessSecurityPolicyImpl::GrantDeleteFromFileSystem(
+    int child_id, const std::string& filesystem_id) {
+  GrantPermissionsForFileSystem(child_id, filesystem_id, DELETE_FILE_GRANT);
+}
+
 void ChildProcessSecurityPolicyImpl::GrantSendMIDISysExMessage(int child_id) {
   base::AutoLock lock(lock_);
 
@@ -627,6 +635,12 @@ bool ChildProcessSecurityPolicyImpl::CanCopyIntoFileSystem(
                                      COPY_INTO_FILE_GRANT);
 }
 
+bool ChildProcessSecurityPolicyImpl::CanDeleteFromFileSystem(
+    int child_id, const std::string& filesystem_id) {
+  return HasPermissionsForFileSystem(child_id, filesystem_id,
+                                     DELETE_FILE_GRANT);
+}
+
 bool ChildProcessSecurityPolicyImpl::HasPermissionsForFile(
     int child_id, const base::FilePath& file, int permissions) {
   base::AutoLock lock(lock_);
@@ -716,6 +730,12 @@ bool ChildProcessSecurityPolicyImpl::CanCopyIntoFileSystemFile(
   return HasPermissionsForFileSystemFile(child_id, url, COPY_INTO_FILE_GRANT);
 }
 
+bool ChildProcessSecurityPolicyImpl::CanDeleteFileSystemFile(
+    int child_id,
+    const fileapi::FileSystemURL& url) {
+  return HasPermissionsForFileSystemFile(child_id, url, DELETE_FILE_GRANT);
+}
+
 bool ChildProcessSecurityPolicyImpl::HasWebUIBindings(int child_id) {
   base::AutoLock lock(lock_);
 
diff --git a/content/browser/child_process_security_policy_impl.h b/content/browser/child_process_security_policy_impl.h
index 53b9443..646b3e6 100644
--- a/content/browser/child_process_security_policy_impl.h
+++ b/content/browser/child_process_security_policy_impl.h
@@ -57,6 +57,9 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
   virtual void GrantCopyIntoFileSystem(
       int child_id,
       const std::string& filesystem_id) OVERRIDE;
+  virtual void GrantDeleteFromFileSystem(
+      int child_id,
+      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 CanCreateReadWriteFile(int child_id,
@@ -68,6 +71,9 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
       const std::string& filesystem_id) OVERRIDE;
   virtual bool CanCopyIntoFileSystem(int child_id,
                                      const std::string& filesystem_id) OVERRIDE;
+  virtual bool CanDeleteFromFileSystem(
+      int child_id,
+      const std::string& filesystem_id) OVERRIDE;
 
   // Pseudo schemes are treated differently than other schemes because they
   // cannot be requested like normal URLs.  There is no mechanism for revoking
@@ -137,6 +143,8 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
                                         const fileapi::FileSystemURL& url);
   bool CanCopyIntoFileSystemFile(int child_id,
                                  const fileapi::FileSystemURL& url);
+  bool CanDeleteFileSystemFile(int child_id,
+                               const fileapi::FileSystemURL& url);
 
   // Returns true if the specified child_id has been granted WebUIBindings.
   // The browser should check this property before assuming the child process is
diff --git a/content/browser/child_process_security_policy_unittest.cc b/content/browser/child_process_security_policy_unittest.cc
index 2e68b18..670848f 100644
--- a/content/browser/child_process_security_policy_unittest.cc
+++ b/content/browser/child_process_security_policy_unittest.cc
@@ -89,6 +89,27 @@ class ChildProcessSecurityPolicyTest : public testing::Test {
     p->GrantPermissionsForFile(child_id, file, permissions);
   }
 
+  void CheckHasNoFileSystemPermission(ChildProcessSecurityPolicyImpl* p,
+                                      const std::string& child_id) {
+    EXPECT_FALSE(p->CanReadFileSystem(kRendererID, child_id));
+    EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, child_id));
+    EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, child_id));
+    EXPECT_FALSE(p->CanDeleteFromFileSystem(kRendererID, child_id));
+  }
+
+  void CheckHasNoFileSystemFilePermission(ChildProcessSecurityPolicyImpl* p,
+                                          const base::FilePath& file,
+                                          const fileapi::FileSystemURL& url) {
+    EXPECT_FALSE(p->CanReadFile(kRendererID, file));
+    EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
+    EXPECT_FALSE(p->CanReadFileSystemFile(kRendererID, url));
+    EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
+    EXPECT_FALSE(p->CanCreateFileSystemFile(kRendererID, url));
+    EXPECT_FALSE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
+    EXPECT_FALSE(p->CanCopyIntoFileSystemFile(kRendererID, url));
+    EXPECT_FALSE(p->CanDeleteFileSystemFile(kRendererID, url));
+  }
+
  private:
   ChildProcessSecurityPolicyTestBrowserClient test_browser_client_;
   ContentBrowserClient* old_browser_client_;
@@ -300,70 +321,63 @@ TEST_F(ChildProcessSecurityPolicyTest, FileSystemGrantsTest) {
       RegisterFileSystemForVirtualPath(fileapi::kFileSystemTypeTest,
                                        "copy_into_filesystem",
                                        base::FilePath());
+  std::string delete_from_id = fileapi::IsolatedContext::GetInstance()->
+      RegisterFileSystemForVirtualPath(fileapi::kFileSystemTypeTest,
+                                       "delete_from_filesystem",
+                                       base::FilePath());
 
   // Test initially having no permissions.
-  EXPECT_FALSE(p->CanReadFileSystem(kRendererID, read_id));
-  EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, read_id));
-  EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, read_id));
-
-  EXPECT_FALSE(p->CanReadFileSystem(kRendererID, read_write_id));
-  EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, read_write_id));
-  EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, read_write_id));
-
-  EXPECT_FALSE(p->CanReadFileSystem(kRendererID, copy_into_id));
-  EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, copy_into_id));
-  EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, copy_into_id));
+  CheckHasNoFileSystemPermission(p, read_id);
+  CheckHasNoFileSystemPermission(p, read_write_id);
+  CheckHasNoFileSystemPermission(p, copy_into_id);
+  CheckHasNoFileSystemPermission(p, delete_from_id);
 
   // Testing varying combinations of grants and checks.
   p->GrantReadFileSystem(kRendererID, read_id);
   EXPECT_TRUE(p->CanReadFileSystem(kRendererID, read_id));
   EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, read_id));
   EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, read_id));
+  EXPECT_FALSE(p->CanDeleteFromFileSystem(kRendererID, read_id));
 
   p->GrantReadFileSystem(kRendererID, read_write_id);
   p->GrantWriteFileSystem(kRendererID, read_write_id);
   EXPECT_TRUE(p->CanReadFileSystem(kRendererID, read_write_id));
   EXPECT_TRUE(p->CanReadWriteFileSystem(kRendererID, read_write_id));
   EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, read_write_id));
+  EXPECT_FALSE(p->CanDeleteFromFileSystem(kRendererID, read_write_id));
 
   p->GrantCopyIntoFileSystem(kRendererID, copy_into_id);
   EXPECT_FALSE(p->CanReadFileSystem(kRendererID, copy_into_id));
   EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, copy_into_id));
   EXPECT_TRUE(p->CanCopyIntoFileSystem(kRendererID, copy_into_id));
+  EXPECT_FALSE(p->CanDeleteFromFileSystem(kRendererID, copy_into_id));
+
+  p->GrantDeleteFromFileSystem(kRendererID, delete_from_id);
+  EXPECT_FALSE(p->CanReadFileSystem(kRendererID, delete_from_id));
+  EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, delete_from_id));
+  EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, delete_from_id));
+  EXPECT_TRUE(p->CanDeleteFromFileSystem(kRendererID, delete_from_id));
 
   // Test revoke permissions on renderer ID removal.
   p->Remove(kRendererID);
-  EXPECT_FALSE(p->CanReadFileSystem(kRendererID, read_id));
-  EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, read_id));
-  EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, read_id));
-
-  EXPECT_FALSE(p->CanReadFileSystem(kRendererID, read_write_id));
-  EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, read_write_id));
-  EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, read_write_id));
-
-  EXPECT_FALSE(p->CanReadFileSystem(kRendererID, copy_into_id));
-  EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, copy_into_id));
-  EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, copy_into_id));
+  CheckHasNoFileSystemPermission(p, read_id);
+  CheckHasNoFileSystemPermission(p, read_write_id);
+  CheckHasNoFileSystemPermission(p, copy_into_id);
+  CheckHasNoFileSystemPermission(p, delete_from_id);
 
   // Test having no permissions upon re-adding same renderer ID.
   p->Add(kRendererID);
-  EXPECT_FALSE(p->CanReadFileSystem(kRendererID, read_id));
-  EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, read_id));
-  EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, read_id));
-
-  EXPECT_FALSE(p->CanReadFileSystem(kRendererID, read_write_id));
-  EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, read_write_id));
-  EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, read_write_id));
-
-  EXPECT_FALSE(p->CanReadFileSystem(kRendererID, copy_into_id));
-  EXPECT_FALSE(p->CanReadWriteFileSystem(kRendererID, copy_into_id));
-  EXPECT_FALSE(p->CanCopyIntoFileSystem(kRendererID, copy_into_id));
+  CheckHasNoFileSystemPermission(p, read_id);
+  CheckHasNoFileSystemPermission(p, read_write_id);
+  CheckHasNoFileSystemPermission(p, copy_into_id);
+  CheckHasNoFileSystemPermission(p, delete_from_id);
 
   // Cleanup.
   p->Remove(kRendererID);
   fileapi::IsolatedContext::GetInstance()->RevokeFileSystem(read_id);
   fileapi::IsolatedContext::GetInstance()->RevokeFileSystem(read_write_id);
   fileapi::IsolatedContext::GetInstance()->RevokeFileSystem(copy_into_id);
+  fileapi::IsolatedContext::GetInstance()->RevokeFileSystem(delete_from_id);
 }
 
 TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
@@ -381,12 +395,7 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
       GURL("http://foo/"), fileapi::kFileSystemTypeTest, file);
 
   // Test initially having no permissions.
-  EXPECT_FALSE(p->CanReadFile(kRendererID, file));
-  EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
-  EXPECT_FALSE(p->CanReadFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanCreateFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
+  CheckHasNoFileSystemFilePermission(p, file, url);
 
   // Testing every combination of permissions granting and revoking.
   p->GrantReadFile(kRendererID, file);
@@ -396,13 +405,10 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
   EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
   EXPECT_FALSE(p->CanCreateFileSystemFile(kRendererID, url));
   EXPECT_FALSE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
+  EXPECT_FALSE(p->CanCopyIntoFileSystemFile(kRendererID, url));
+  EXPECT_FALSE(p->CanDeleteFileSystemFile(kRendererID, url));
   p->RevokeAllPermissionsForFile(kRendererID, file);
-  EXPECT_FALSE(p->CanReadFile(kRendererID, file));
-  EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
-  EXPECT_FALSE(p->CanReadFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanCreateFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
+  CheckHasNoFileSystemFilePermission(p, file, url);
 
   p->GrantCreateReadWriteFile(kRendererID, file);
   EXPECT_TRUE(p->CanReadFile(kRendererID, file));
@@ -411,13 +417,10 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
   EXPECT_TRUE(p->CanWriteFileSystemFile(kRendererID, url));
   EXPECT_TRUE(p->CanCreateFileSystemFile(kRendererID, url));
   EXPECT_TRUE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
+  EXPECT_TRUE(p->CanCopyIntoFileSystemFile(kRendererID, url));
+  EXPECT_TRUE(p->CanDeleteFileSystemFile(kRendererID, url));
   p->RevokeAllPermissionsForFile(kRendererID, file);
-  EXPECT_FALSE(p->CanReadFile(kRendererID, file));
-  EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
-  EXPECT_FALSE(p->CanReadFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanCreateFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
+  CheckHasNoFileSystemFilePermission(p, file, url);
 
   // Test revoke permissions on renderer ID removal.
   p->GrantCreateReadWriteFile(kRendererID, file);
@@ -427,22 +430,14 @@ TEST_F(ChildProcessSecurityPolicyTest, FilePermissionGrantingAndRevoking) {
   EXPECT_TRUE(p->CanWriteFileSystemFile(kRendererID, url));
   EXPECT_TRUE(p->CanCreateFileSystemFile(kRendererID, url));
   EXPECT_TRUE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
+  EXPECT_TRUE(p->CanCopyIntoFileSystemFile(kRendererID, url));
+  EXPECT_TRUE(p->CanDeleteFileSystemFile(kRendererID, url));
   p->Remove(kRendererID);
-  EXPECT_FALSE(p->CanReadFile(kRendererID, file));
-  EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
-  EXPECT_FALSE(p->CanReadFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanCreateFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
+  CheckHasNoFileSystemFilePermission(p, file, url);
 
   // Test having no permissions upon re-adding same renderer ID.
   p->Add(kRendererID);
-  EXPECT_FALSE(p->CanReadFile(kRendererID, file));
-  EXPECT_FALSE(p->CanCreateReadWriteFile(kRendererID, file));
-  EXPECT_FALSE(p->CanReadFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanWriteFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanCreateFileSystemFile(kRendererID, url));
-  EXPECT_FALSE(p->CanCreateReadWriteFileSystemFile(kRendererID, url));
+  CheckHasNoFileSystemFilePermission(p, file, url);
 
   // Cleanup.
   p->Remove(kRendererID);
diff --git a/content/browser/fileapi/fileapi_message_filter.cc b/content/browser/fileapi/fileapi_message_filter.cc
index 98deb77..5c1282c9 100644
--- a/content/browser/fileapi/fileapi_message_filter.cc
+++ b/content/browser/fileapi/fileapi_message_filter.cc
@@ -284,7 +284,7 @@ void FileAPIMessageFilter::OnMove(
     return;
   }
   if (!security_policy_->CanReadFileSystemFile(process_id_, src_url) ||
-      !security_policy_->CanWriteFileSystemFile(process_id_, src_url) ||
+      !security_policy_->CanDeleteFileSystemFile(process_id_, src_url) ||
       !security_policy_->CanCreateFileSystemFile(process_id_, dest_url)) {
     Send(new FileSystemMsg_DidFail(request_id,
                                    base::PLATFORM_FILE_ERROR_SECURITY));
@@ -326,7 +326,7 @@ void FileAPIMessageFilter::OnRemove(
   FileSystemURL url(context_->CrackURL(path));
   if (!ValidateFileSystemURL(request_id, url))
     return;
-  if (!security_policy_->CanWriteFileSystemFile(process_id_, url)) {
+  if (!security_policy_->CanDeleteFileSystemFile(process_id_, url)) {
     Send(new FileSystemMsg_DidFail(request_id,
                                    base::PLATFORM_FILE_ERROR_SECURITY));
     return;
diff --git a/content/public/browser/child_process_security_policy.h b/content/public/browser/child_process_security_policy.h
index 22a3d7e..6807690 100644
--- a/content/public/browser/child_process_security_policy.h
+++ b/content/public/browser/child_process_security_policy.h
@@ -107,6 +107,12 @@ class ChildProcessSecurityPolicy {
   virtual void GrantCopyIntoFileSystem(int child_id,
                                        const std::string& filesystem_id) = 0;
 
+  // Grants permission to delete from filesystem |filesystem_id|. 'delete-from'
+  // is used to allow deleting files into the destination filesystem without
+  // granting more general create and write permissions.
+  virtual void GrantDeleteFromFileSystem(int child_id,
+                                         const std::string& filesystem_id) = 0;
+
   // Grants the child process the capability to access URLs of the provided
   // scheme.
   virtual void GrantScheme(int child_id, const std::string& scheme) = 0;
@@ -122,6 +128,10 @@ class ChildProcessSecurityPolicy {
   // Returns true if copy-into access has been granted to |filesystem_id|.
   virtual bool CanCopyIntoFileSystem(int child_id,
                                      const std::string& filesystem_id) = 0;
+
+  // Returns true if delete-from access has been granted to |filesystem_id|.
+  virtual bool CanDeleteFromFileSystem(int child_id,
+                                       const std::string& filesystem_id) = 0;
 };
 
 };  // namespace content