Make Isolated file system writable only if it is configured so

- Use CreateFileStreamWriter in FSO
- Add yet another write-protection at IsolatedContext (in addition to ChildProcessSecurityPolicy, so that we can make each fs read-only or writable in DRT/testing)

BUG=none
TEST=none (will add layout_test)

Review URL: https://chromiumcodereview.appspot.com/10540070

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

9 files changed, 104 insertions, 34 deletions

diff --git a/content/browser/child_process_security_policy_impl.cc b/content/browser/child_process_security_policy_impl.cc
index 1f40966..90c0ead 100644
--- a/content/browser/child_process_security_policy_impl.cc
+++ b/content/browser/child_process_security_policy_impl.cc
@@ -399,6 +399,7 @@ void ChildProcessSecurityPolicyImpl::GrantReadFileSystem(
 
 void ChildProcessSecurityPolicyImpl::GrantReadWriteFileSystem(
     int child_id, const std::string& filesystem_id) {
+  fileapi::IsolatedContext::GetInstance()->SetWritable(filesystem_id, true);
   GrantPermissionsForFileSystem(child_id, filesystem_id,
                                 kReadFilePermissions |
                                 kWriteFilePermissions);
diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc
index 1084fd8..5c77024 100644
--- a/webkit/fileapi/file_system_operation.cc
+++ b/webkit/fileapi/file_system_operation.cc
@@ -287,13 +287,25 @@ void FileSystemOperation::Write(
     delete this;
     return;
   }
+
+  FileSystemMountPointProvider* provider = file_system_context()->
+      GetMountPointProvider(src_path_.type());
+  DCHECK(provider);
+  scoped_ptr<FileStreamWriter> writer(provider->CreateFileStreamWriter(
+          path_url, offset, file_system_context()));
+
+  if (!writer.get()) {
+    // Write is not supported.
+    callback.Run(base::PLATFORM_FILE_ERROR_SECURITY, 0, false);
+    delete this;
+    return;
+  }
+
   DCHECK(blob_url.is_valid());
   file_writer_delegate_.reset(new FileWriterDelegate(
       base::Bind(&FileSystemOperation::DidWrite, weak_factory_.GetWeakPtr()),
-      scoped_ptr<FileStreamWriter>(
-          new SandboxFileStreamWriter(file_system_context(),
-                                      path_url,
-                                      offset))));
+      writer.Pass()));
+
   set_write_callback(callback);
   scoped_ptr<net::URLRequest> blob_request(
       new net::URLRequest(blob_url, file_writer_delegate_.get()));
diff --git a/webkit/fileapi/isolated_context.cc b/webkit/fileapi/isolated_context.cc
index cc9e925..e8b52ae 100644
--- a/webkit/fileapi/isolated_context.cc
+++ b/webkit/fileapi/isolated_context.cc
@@ -51,6 +51,7 @@ void IsolatedContext::RevokeIsolatedFileSystem(
     const std::string& filesystem_id) {
   base::AutoLock locker(lock_);
   toplevel_map_.erase(filesystem_id);
+  writable_ids_.erase(filesystem_id);
 }
 
 bool IsolatedContext::CrackIsolatedPath(const FilePath& virtual_path,
@@ -97,7 +98,7 @@ bool IsolatedContext::CrackIsolatedPath(const FilePath& virtual_path,
   return true;
 }
 
-bool IsolatedContext::GetTopLevelPaths(std::string filesystem_id,
+bool IsolatedContext::GetTopLevelPaths(const std::string& filesystem_id,
                                        std::vector<FilePath>* paths) const {
   DCHECK(paths);
   base::AutoLock locker(lock_);
@@ -114,6 +115,23 @@ bool IsolatedContext::GetTopLevelPaths(std::string filesystem_id,
   return true;
 }
 
+bool IsolatedContext::SetWritable(const std::string& filesystem_id,
+                                  bool writable) {
+  base::AutoLock locker(lock_);
+  if (toplevel_map_.find(filesystem_id) == toplevel_map_.end())
+    return false;
+  if (writable)
+    writable_ids_.insert(filesystem_id);
+  else
+    writable_ids_.erase(filesystem_id);
+  return true;
+}
+
+bool IsolatedContext::IsWritable(const std::string& filesystem_id) const {
+  base::AutoLock locker(lock_);
+  return (writable_ids_.find(filesystem_id) != writable_ids_.end());
+}
+
 FilePath IsolatedContext::CreateVirtualPath(
     const std::string& filesystem_id, const FilePath& relative_path) const {
   FilePath full_path;
diff --git a/webkit/fileapi/isolated_context.h b/webkit/fileapi/isolated_context.h
index 6024ab3..fa6d0a9 100644
--- a/webkit/fileapi/isolated_context.h
+++ b/webkit/fileapi/isolated_context.h
@@ -77,13 +77,20 @@ class FILEAPI_EXPORT IsolatedContext {
   // Returns a vector of the full paths of the top-level entry paths
   // registered for the |filesystem_id|.  Returns false if the
   // |filesystem_is| is not valid.
-  bool GetTopLevelPaths(std::string filesystem_id,
+  bool GetTopLevelPaths(const std::string& filesystem_id,
                         std::vector<FilePath>* paths) const;
 
   // Returns the virtual path that looks like /<filesystem_id>/<relative_path>.
   FilePath CreateVirtualPath(const std::string& filesystem_id,
                              const FilePath& relative_path) const;
 
+  // Set the filesystem writable if |writable| is true, non-writable
+  // if it is false. Returns false if the |filesystem_id| is not valid.
+  bool SetWritable(const std::string& filesystem_id, bool writable);
+
+  // Returns true if the |filesystem_id| is writable.
+  bool IsWritable(const std::string& filesystem_id) const;
+
  private:
   friend struct base::DefaultLazyInstanceTraits<IsolatedContext>;
 
@@ -104,6 +111,13 @@ class FILEAPI_EXPORT IsolatedContext {
   // Maps the toplevel entries to the filesystem id.
   IDToPathMap toplevel_map_;
 
+  // Holds a set of writable ids.
+  // Isolated file systems are created read-only by default, and this set
+  // holds a list of exceptions.
+  // Detailed filesystem permission may be provided by an external
+  // security policy manager, e.g. ChildProcessSecurityPolicy.
+  std::set<std::string> writable_ids_;
+
   DISALLOW_COPY_AND_ASSIGN(IsolatedContext);
 };
 
diff --git a/webkit/fileapi/isolated_context_unittest.cc b/webkit/fileapi/isolated_context_unittest.cc
index 5a828f5..a1b138b 100644
--- a/webkit/fileapi/isolated_context_unittest.cc
+++ b/webkit/fileapi/isolated_context_unittest.cc
@@ -165,4 +165,28 @@ TEST_F(IsolatedContextTest, TestWithVirtualRoot) {
       virtual_path, &cracked_id, &root_path, &cracked_path));
 }
 
+TEST_F(IsolatedContextTest, Writable) {
+  // By default the file system must be read-only.
+  ASSERT_FALSE(isolated_context()->IsWritable(id_));
+
+  // Set writable.
+  ASSERT_TRUE(isolated_context()->SetWritable(id_, true));
+  ASSERT_TRUE(isolated_context()->IsWritable(id_));
+
+  // Set non-writable.
+  ASSERT_TRUE(isolated_context()->SetWritable(id_, false));
+  ASSERT_FALSE(isolated_context()->IsWritable(id_));
+
+  // Set writable again, and revoke the filesystem.
+  ASSERT_TRUE(isolated_context()->SetWritable(id_, true));
+  isolated_context()->RevokeIsolatedFileSystem(id_);
+
+  // IsWritable should return false for non-registered file system.
+  ASSERT_FALSE(isolated_context()->IsWritable(id_));
+  // SetWritable should also return false for non-registered file system
+  // (no matter what value we give).
+  ASSERT_FALSE(isolated_context()->SetWritable(id_, true));
+  ASSERT_FALSE(isolated_context()->SetWritable(id_, false));
+}
+
 }  // namespace fileapi
diff --git a/webkit/fileapi/isolated_mount_point_provider.cc b/webkit/fileapi/isolated_mount_point_provider.cc
index 60ad91e..9414211 100644
--- a/webkit/fileapi/isolated_mount_point_provider.cc
+++ b/webkit/fileapi/isolated_mount_point_provider.cc
@@ -26,6 +26,29 @@
 
 namespace fileapi {
 
+namespace {
+
+IsolatedContext* isolated_context() {
+  return IsolatedContext::GetInstance();
+}
+
+FilePath GetPathFromURL(const GURL& url, bool for_writing) {
+  GURL origin_url;
+  FileSystemType file_system_type = kFileSystemTypeUnknown;
+  FilePath virtual_path;
+  if (!CrackFileSystemURL(url, &origin_url, &file_system_type, &virtual_path))
+    return FilePath();
+  std::string fsid;
+  FilePath path;
+  if (!isolated_context()->CrackIsolatedPath(virtual_path, &fsid, NULL, &path))
+    return FilePath();
+  if (for_writing && !isolated_context()->IsWritable(fsid))
+    return FilePath();
+  return path;
+}
+
+}  // namespace
+
 IsolatedMountPointProvider::IsolatedMountPointProvider()
     : isolated_file_util_(new IsolatedFileUtil()) {
 }
@@ -107,7 +130,7 @@ IsolatedMountPointProvider::CreateFileStreamReader(
     const GURL& url,
     int64 offset,
     FileSystemContext* context) const {
-  FilePath path = GetPathFromURL(url);
+  FilePath path = GetPathFromURL(url, false);
   return path.empty() ? NULL : new webkit_blob::LocalFileStreamReader(
       context->file_task_runner(), path, offset, base::Time());
 }
@@ -116,7 +139,7 @@ FileStreamWriter* IsolatedMountPointProvider::CreateFileStreamWriter(
     const GURL& url,
     int64 offset,
     FileSystemContext* context) const {
-  FilePath path = GetPathFromURL(url);
+  FilePath path = GetPathFromURL(url, true);
   return path.empty() ? NULL : new LocalFileStreamWriter(path, offset);
 }
 
@@ -125,21 +148,4 @@ FileSystemQuotaUtil* IsolatedMountPointProvider::GetQuotaUtil() {
   return NULL;
 }
 
-IsolatedContext* IsolatedMountPointProvider::isolated_context() const {
-  return IsolatedContext::GetInstance();
-}
-
-FilePath IsolatedMountPointProvider::GetPathFromURL(const GURL& url) const {
-  GURL origin_url;
-  FileSystemType file_system_type = kFileSystemTypeUnknown;
-  FilePath virtual_path;
-  if (!CrackFileSystemURL(url, &origin_url, &file_system_type, &virtual_path))
-    return FilePath();
-  std::string fsid;
-  FilePath path;
-  if (!isolated_context()->CrackIsolatedPath(virtual_path, &fsid, NULL, &path))
-    return FilePath();
-  return path;
-}
-
 }  // namespace fileapi
diff --git a/webkit/fileapi/isolated_mount_point_provider.h b/webkit/fileapi/isolated_mount_point_provider.h
index a071cc9..08f6b07 100644
--- a/webkit/fileapi/isolated_mount_point_provider.h
+++ b/webkit/fileapi/isolated_mount_point_provider.h
@@ -58,9 +58,6 @@ class IsolatedMountPointProvider : public FileSystemMountPointProvider {
   virtual FileSystemQuotaUtil* GetQuotaUtil() OVERRIDE;
 
  private:
-  IsolatedContext* isolated_context() const;
-  FilePath GetPathFromURL(const GURL& url) const;
-
   scoped_ptr<IsolatedFileUtil> isolated_file_util_;
 };
 
diff --git a/webkit/fileapi/sandbox_mount_point_provider.cc b/webkit/fileapi/sandbox_mount_point_provider.cc
index 0c1f59f..46d734b 100644
--- a/webkit/fileapi/sandbox_mount_point_provider.cc
+++ b/webkit/fileapi/sandbox_mount_point_provider.cc
@@ -24,6 +24,7 @@
 #include "webkit/fileapi/file_system_util.h"
 #include "webkit/fileapi/native_file_util.h"
 #include "webkit/fileapi/obfuscated_file_util.h"
+#include "webkit/fileapi/sandbox_file_stream_writer.h"
 #include "webkit/glue/webkit_glue.h"
 #include "webkit/quota/quota_manager.h"
 
@@ -461,9 +462,7 @@ fileapi::FileStreamWriter* SandboxMountPointProvider::CreateFileStreamWriter(
     const GURL& url,
     int64 offset,
     FileSystemContext* context) const {
-  // TODO(kinaba,kinuko): return SandboxFileWriter when it is implemented.
-  NOTIMPLEMENTED();
-  return NULL;
+  return new SandboxFileStreamWriter(context, url, offset);
 }
 
 FileSystemQuotaUtil* SandboxMountPointProvider::GetQuotaUtil() {
diff --git a/webkit/fileapi/test_mount_point_provider.cc b/webkit/fileapi/test_mount_point_provider.cc
index 20e716d..ce8dfc8 100644
--- a/webkit/fileapi/test_mount_point_provider.cc
+++ b/webkit/fileapi/test_mount_point_provider.cc
@@ -16,6 +16,7 @@
 #include "webkit/fileapi/file_system_util.h"
 #include "webkit/fileapi/local_file_util.h"
 #include "webkit/fileapi/native_file_util.h"
+#include "webkit/fileapi/sandbox_file_stream_writer.h"
 #include "webkit/quota/quota_manager.h"
 
 namespace fileapi {
@@ -156,9 +157,7 @@ fileapi::FileStreamWriter* TestMountPointProvider::CreateFileStreamWriter(
     const GURL& url,
     int64 offset,
     FileSystemContext* context) const {
-  // TODO(kinaba,kinuko): return SandboxFileWriter when it is implemented.
-  NOTIMPLEMENTED();
-  return NULL;
+  return new SandboxFileStreamWriter(context, url, offset);
 }
 
 FileSystemQuotaUtil* TestMountPointProvider::GetQuotaUtil() {