Improve Storage Service sandboxing support

This corrects some deficiencies in the Storage Service's sandboxing
support by eliminating all remaining instances of direct filesystem
traversal within DOM Storage, replacing them with appropriate
FilesystemProxy usage.

A few new IPCs are added to the Directory mojom interface in support of
this, and a new delegate is added to support use of
sql::SandboxedVfs with a FilesystemProxy backing it.

Bug: 1052045
Test: content_browsertests with StorageServiceOutOfProcess and StorageServiceSandbox enabled
Change-Id: I8e7593d9424be705cb3c2bf561a4fe4c5d61251d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2350542
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798710}

components/services/storage/BUILD.gn

@@ -67,6 +67,8 @@ source_set("storage") {
     "origin_context_impl.h",
     "partition_impl.cc",
     "partition_impl.h",
+    "sandboxed_vfs_delegate.cc",
+    "sandboxed_vfs_delegate.h",
     "storage_service_impl.cc",
     "storage_service_impl.h",
   ]

components/services/storage/DEPS

 include_rules = [
   "+third_party/blink/public/common",
   "+third_party/leveldatabase",
+  "+sql",
 ]

components/services/storage/dom_storage/legacy_dom_storage_database.cc

@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
+#include "components/services/storage/public/cpp/filesystem/filesystem_proxy.h"
 #include "sql/statement.h"
 #include "sql/transaction.h"
 #include "third_party/sqlite/sqlite3.h"
@@ -14,15 +15,18 @@
 namespace storage {
 
 LegacyDomStorageDatabase::LegacyDomStorageDatabase(
-    const base::FilePath& file_path)
-    : file_path_(file_path) {
+    const base::FilePath& file_path,
+    std::unique_ptr<FilesystemProxy> filesystem_proxy)
+    : file_path_(file_path), filesystem_proxy_(std::move(filesystem_proxy)) {
   // Note: in normal use we should never get an empty backing path here.
   // However, the unit test for this class can contruct an instance
   // with an empty path.
   Init();
 }
 
-LegacyDomStorageDatabase::LegacyDomStorageDatabase() {
+LegacyDomStorageDatabase::LegacyDomStorageDatabase(
+    std::unique_ptr<FilesystemProxy> filesystem_proxy)
+    : filesystem_proxy_(std::move(filesystem_proxy)) {
   Init();
 }
 
@@ -67,7 +71,8 @@ bool LegacyDomStorageDatabase::CommitChanges(
   if (!LazyOpen(!changes.empty())) {
     // If we're being asked to commit changes that will result in an
     // empty database, we return true if the database file doesn't exist.
-    return clear_all_first && changes.empty() && !base::PathExists(file_path_);
+    return clear_all_first && changes.empty() &&
+           !filesystem_proxy_->PathExists(file_path_);
   }
 
   bool old_known_to_be_empty = known_to_be_empty_;
@@ -140,7 +145,7 @@ bool LegacyDomStorageDatabase::LazyOpen(bool create_if_needed) {
   if (IsOpen())
     return true;
 
-  bool database_exists = base::PathExists(file_path_);
+  bool database_exists = filesystem_proxy_->PathExists(file_path_);
 
   if (!database_exists && !create_if_needed) {
     // If the file doesn't exist already and we haven't been asked to create
@@ -230,7 +235,7 @@ bool LegacyDomStorageDatabase::CreateTableV2() {
 
 bool LegacyDomStorageDatabase::DeleteFileAndRecreate() {
   DCHECK(!IsOpen());
-  DCHECK(base::PathExists(file_path_));
+  DCHECK(filesystem_proxy_->PathExists(file_path_));
 
   // We should only try and do this once.
   if (tried_to_recreate_)
@@ -238,8 +243,10 @@ bool LegacyDomStorageDatabase::DeleteFileAndRecreate() {
 
   tried_to_recreate_ = true;
 
+  base::Optional<base::File::Info> info =
+      filesystem_proxy_->GetFileInfo(file_path_);
   // If it's not a directory and we can delete the file, try and open it again.
-  if (!base::DirectoryExists(file_path_) && sql::Database::Delete(file_path_)) {
+  if (info && !info->is_directory && sql::Database::Delete(file_path_)) {
     return LazyOpen(true);
   }
 

components/services/storage/dom_storage/legacy_dom_storage_database.h

@@ -23,6 +23,8 @@ class ProcessMemoryDump;
 
 namespace storage {
 
+class FilesystemProxy;
+
 using LegacyDomStorageValuesMap =
     std::map<base::string16, base::NullableString16>;
 
@@ -30,7 +32,8 @@ using LegacyDomStorageValuesMap =
 // class is designed to be used on a single thread.
 class LegacyDomStorageDatabase {
  public:
-  explicit LegacyDomStorageDatabase(const base::FilePath& file_path);
+  LegacyDomStorageDatabase(const base::FilePath& file_path,
+                           std::unique_ptr<FilesystemProxy> filesystem_proxy);
   virtual ~LegacyDomStorageDatabase();  // virtual for unit testing
 
   // Reads all the key, value pairs stored in the database and returns
@@ -56,7 +59,8 @@ class LegacyDomStorageDatabase {
 
  protected:
   // Constructor that uses an in-memory sqlite database, for testing.
-  LegacyDomStorageDatabase();
+  explicit LegacyDomStorageDatabase(
+      std::unique_ptr<FilesystemProxy> filesystem_proxy);
 
  private:
   friend class LocalStorageDatabaseAdapter;
@@ -123,6 +127,7 @@ class LegacyDomStorageDatabase {
 
   // Path to the database on disk.
   const base::FilePath file_path_;
+  const std::unique_ptr<FilesystemProxy> filesystem_proxy_;
   std::unique_ptr<sql::Database> db_;
   bool failed_to_open_;
   bool tried_to_recreate_;

components/services/storage/dom_storage/legacy_dom_storage_database_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/services/storage/public/cpp/filesystem/filesystem_proxy.h"
 #include "sql/statement.h"
 #include "sql/test/scoped_error_expecter.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -17,6 +18,15 @@ using base::ASCIIToUTF16;
 
 namespace storage {
 
+namespace {
+
+std::unique_ptr<FilesystemProxy> MakeFilesystemProxy(
+    const base::FilePath& root = base::FilePath()) {
+  return std::make_unique<FilesystemProxy>(FilesystemProxy::UNRESTRICTED, root);
+}
+
+}  // namespace
+
 void CreateV2Table(sql::Database* db) {
   ASSERT_TRUE(db->is_open());
   ASSERT_TRUE(db->Execute("DROP TABLE IF EXISTS ItemTable"));
@@ -65,7 +75,7 @@ void CreateMapWithValues(LegacyDomStorageValuesMap* values) {
 }
 
 TEST(LegacyDomStorageDatabaseTest, SimpleOpenAndClose) {
-  LegacyDomStorageDatabase db;
+  LegacyDomStorageDatabase db(MakeFilesystemProxy());
   EXPECT_FALSE(db.IsOpen());
   ASSERT_TRUE(db.LazyOpen(true));
   EXPECT_TRUE(db.IsOpen());
@@ -85,7 +95,8 @@ TEST(LegacyDomStorageDatabaseTest, CloseEmptyDatabaseDeletesFile) {
   // First test the case that explicitly clearing the database will
   // trigger its deletion from disk.
   {
-    LegacyDomStorageDatabase db(file_name);
+    LegacyDomStorageDatabase db(file_name,
+                                MakeFilesystemProxy(temp_dir.GetPath()));
     EXPECT_EQ(file_name, db.file_path());
     ASSERT_TRUE(db.CommitChanges(false, storage));
   }
@@ -94,7 +105,8 @@ TEST(LegacyDomStorageDatabaseTest, CloseEmptyDatabaseDeletesFile) {
   {
     // Check that reading an existing db with data in it
     // keeps the DB on disk on close.
-    LegacyDomStorageDatabase db(file_name);
+    LegacyDomStorageDatabase db(file_name,
+                                MakeFilesystemProxy(temp_dir.GetPath()));
     LegacyDomStorageValuesMap values;
     db.ReadAllValues(&values);
     EXPECT_EQ(storage.size(), values.size());
@@ -104,7 +116,8 @@ TEST(LegacyDomStorageDatabaseTest, CloseEmptyDatabaseDeletesFile) {
   storage.clear();
 
   {
-    LegacyDomStorageDatabase db(file_name);
+    LegacyDomStorageDatabase db(file_name,
+                                MakeFilesystemProxy(temp_dir.GetPath()));
     ASSERT_TRUE(db.CommitChanges(true, storage));
   }
   EXPECT_FALSE(base::PathExists(file_name));
@@ -113,14 +126,16 @@ TEST(LegacyDomStorageDatabaseTest, CloseEmptyDatabaseDeletesFile) {
   // is an empty database also triggers deletion.
   CreateMapWithValues(&storage);
   {
-    LegacyDomStorageDatabase db(file_name);
+    LegacyDomStorageDatabase db(file_name,
+                                MakeFilesystemProxy(temp_dir.GetPath()));
     ASSERT_TRUE(db.CommitChanges(false, storage));
   }
 
   EXPECT_TRUE(base::PathExists(file_name));
 
   {
-    LegacyDomStorageDatabase db(file_name);
+    LegacyDomStorageDatabase db(file_name,
+                                MakeFilesystemProxy(temp_dir.GetPath()));
     ASSERT_TRUE(db.CommitChanges(false, storage));
     auto it = storage.begin();
     for (; it != storage.end(); ++it)
@@ -138,7 +153,8 @@ TEST(LegacyDomStorageDatabaseTest, TestLazyOpenIsLazy) {
   base::FilePath file_name =
       temp_dir.GetPath().AppendASCII("TestLegacyDomStorageDatabase.db");
 
-  LegacyDomStorageDatabase db(file_name);
+  LegacyDomStorageDatabase db(file_name,
+                              MakeFilesystemProxy(temp_dir.GetPath()));
   EXPECT_FALSE(db.IsOpen());
   LegacyDomStorageValuesMap values;
   db.ReadAllValues(&values);
@@ -160,8 +176,8 @@ TEST(LegacyDomStorageDatabaseTest, TestLazyOpenIsLazy) {
 }
 
 TEST(LegacyDomStorageDatabaseTest, TestDetectSchemaVersion) {
-  LegacyDomStorageDatabase db;
-  db.db_.reset(new sql::Database());
+  LegacyDomStorageDatabase db(MakeFilesystemProxy());
+  db.db_ = std::make_unique<sql::Database>();
   ASSERT_TRUE(db.db_->OpenInMemory());
 
   CreateInvalidTable(db.db_.get());
@@ -172,7 +188,7 @@ TEST(LegacyDomStorageDatabaseTest, TestDetectSchemaVersion) {
 }
 
 TEST(LegacyDomStorageDatabaseTest, SimpleWriteAndReadBack) {
-  LegacyDomStorageDatabase db;
+  LegacyDomStorageDatabase db(MakeFilesystemProxy());
 
   LegacyDomStorageValuesMap storage;
   CreateMapWithValues(&storage);
@@ -182,7 +198,7 @@ TEST(LegacyDomStorageDatabaseTest, SimpleWriteAndReadBack) {
 }
 
 TEST(LegacyDomStorageDatabaseTest, WriteWithClear) {
-  LegacyDomStorageDatabase db;
+  LegacyDomStorageDatabase db(MakeFilesystemProxy());
 
   LegacyDomStorageValuesMap storage;
   CreateMapWithValues(&storage);
@@ -204,7 +220,7 @@ TEST(LegacyDomStorageDatabaseTest, WriteWithClear) {
 }
 
 TEST(LegacyDomStorageDatabaseTest, TestSimpleRemoveOneValue) {
-  LegacyDomStorageDatabase db;
+  LegacyDomStorageDatabase db(MakeFilesystemProxy());
 
   ASSERT_TRUE(db.LazyOpen(true));
   const base::string16 kCannedKey = ASCIIToUTF16("test");
@@ -244,7 +260,8 @@ TEST(LegacyDomStorageDatabaseTest, TestCanOpenAndReadWebCoreDatabase) {
       temp_dir.GetPath().AppendASCII("dom_storage");
   ASSERT_TRUE(base::CopyFile(test_data, webcore_database));
 
-  LegacyDomStorageDatabase db(webcore_database);
+  LegacyDomStorageDatabase db(webcore_database,
+                              MakeFilesystemProxy(temp_dir.GetPath()));
   LegacyDomStorageValuesMap values;
   db.ReadAllValues(&values);
   EXPECT_TRUE(db.IsOpen());
@@ -280,7 +297,8 @@ TEST(LegacyDomStorageDatabaseTest, TestCanOpenFileThatIsNotADatabase) {
     // Try and open the file. As it's not a database, we should end up deleting
     // it and creating a new, valid file, so everything should actually
     // succeed.
-    LegacyDomStorageDatabase db(file_name);
+    LegacyDomStorageDatabase db(file_name,
+                                MakeFilesystemProxy(temp_dir.GetPath()));
     LegacyDomStorageValuesMap values;
     CreateMapWithValues(&values);
     EXPECT_TRUE(db.CommitChanges(true, values));
@@ -298,7 +316,8 @@ TEST(LegacyDomStorageDatabaseTest, TestCanOpenFileThatIsNotADatabase) {
 
     // Try to open a directory, we should fail gracefully and not attempt
     // to delete it.
-    LegacyDomStorageDatabase db(temp_dir.GetPath());
+    LegacyDomStorageDatabase db(temp_dir.GetPath(),
+                                MakeFilesystemProxy(temp_dir.GetPath()));
     LegacyDomStorageValuesMap values;
     CreateMapWithValues(&values);
     EXPECT_FALSE(db.CommitChanges(true, values));

components/services/storage/dom_storage/local_storage_impl.cc

@@ -38,6 +38,7 @@
 #include "components/services/storage/dom_storage/legacy_dom_storage_database.h"
 #include "components/services/storage/dom_storage/local_storage_database.pb.h"
 #include "components/services/storage/dom_storage/storage_area_impl.h"
+#include "components/services/storage/filesystem_proxy_factory.h"
 #include "components/services/storage/public/cpp/constants.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
 #include "sql/database.h"
@@ -121,7 +122,7 @@ void MigrateStorageHelper(
     const scoped_refptr<base::SingleThreadTaskRunner> reply_task_runner,
     base::OnceCallback<void(std::unique_ptr<StorageAreaImpl::ValueMap>)>
         callback) {
-  LegacyDomStorageDatabase db(db_path);
+  LegacyDomStorageDatabase db(db_path, CreateFilesystemProxy());
   LegacyDomStorageValuesMap map;
   db.ReadAllValues(&map);
   auto values = std::make_unique<StorageAreaImpl::ValueMap>();
@@ -214,17 +215,22 @@ const base::FilePath::CharType kLegacyDatabaseFileExtension[] =
 
 std::vector<mojom::LocalStorageUsageInfoPtr> GetLegacyLocalStorageUsage(
     const base::FilePath& directory) {
+  std::unique_ptr<FilesystemProxy> fs = CreateFilesystemProxy();
+  FileErrorOr<std::vector<base::FilePath>> result = fs->GetDirectoryEntries(
+      directory, FilesystemProxy::DirectoryEntryType::kFilesOnly);
+  if (result.is_error())
+    return {};
+
   std::vector<mojom::LocalStorageUsageInfoPtr> infos;
-  base::FileEnumerator enumerator(directory, false,
-                                  base::FileEnumerator::FILES);
-  for (base::FilePath path = enumerator.Next(); !path.empty();
-       path = enumerator.Next()) {
-    if (path.MatchesExtension(kLegacyDatabaseFileExtension)) {
-      base::FileEnumerator::FileInfo find_info = enumerator.GetInfo();
-      infos.push_back(mojom::LocalStorageUsageInfo::New(
-          LocalStorageImpl::OriginFromLegacyDatabaseFileName(path),
-          find_info.GetSize(), find_info.GetLastModifiedTime()));
-    }
+  for (const auto& path : result.value()) {
+    if (!path.MatchesExtension(kLegacyDatabaseFileExtension))
+      continue;
+    base::Optional<base::File::Info> info = fs->GetFileInfo(path);
+    if (!info)
+      continue;
+    infos.push_back(mojom::LocalStorageUsageInfo::New(
+        LocalStorageImpl::OriginFromLegacyDatabaseFileName(path), info->size,
+        info->last_modified));
   }
   return infos;
 }

components/services/storage/dom_storage/local_storage_impl_unittest.cc

@@ -20,6 +20,7 @@
 #include "components/services/storage/dom_storage/legacy_dom_storage_database.h"
 #include "components/services/storage/dom_storage/storage_area_test_util.h"
 #include "components/services/storage/public/cpp/constants.h"
+#include "components/services/storage/public/cpp/filesystem/filesystem_proxy.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/leveldatabase/env_chromium.h"
@@ -685,7 +686,9 @@ TEST_F(LocalStorageImplTest, Migration) {
   const base::FilePath old_db_path = local_storage_path.Append(
       LocalStorageImpl::LegacyDatabaseFileNameFromOrigin(origin1));
   {
-    LegacyDomStorageDatabase db(old_db_path);
+    LegacyDomStorageDatabase db(
+        old_db_path, std::make_unique<FilesystemProxy>(
+                         FilesystemProxy::UNRESTRICTED, local_storage_path));
     LegacyDomStorageValuesMap data;
     data[key] = base::NullableString16(value, false);
     data[key2] = base::NullableString16(value, false);

components/services/storage/partition_impl.cc

@@ -80,7 +80,8 @@ void PartitionImpl::BindLocalStorageControl(
   local_storage_ = new LocalStorageImpl(
       path_.value_or(base::FilePath()), base::SequencedTaskRunnerHandle::Get(),
       base::ThreadPool::CreateSequencedTaskRunner(
-          {base::MayBlock(), base::TaskShutdownBehavior::BLOCK_SHUTDOWN}),
+          {base::MayBlock(), base::WithBaseSyncPrimitives(),
+           base::TaskShutdownBehavior::BLOCK_SHUTDOWN}),
       std::move(receiver));
 }
 

components/services/storage/public/cpp/filesystem/filesystem_impl.cc

@@ -225,6 +225,11 @@ void FilesystemImpl::GetFileInfo(const base::FilePath& path,
     std::move(callback).Run(base::nullopt);
 }
 
+void FilesystemImpl::GetPathAccess(const base::FilePath& path,
+                                   GetPathAccessCallback callback) {
+  std::move(callback).Run(GetPathAccessLocal(MakeAbsolute(path)));
+}
+
 void FilesystemImpl::RenameFile(const base::FilePath& old_path,
                                 const base::FilePath& new_path,
                                 RenameFileCallback callback) {
@@ -249,6 +254,13 @@ void FilesystemImpl::LockFile(const base::FilePath& path,
   std::move(callback).Run(base::File::FILE_OK, std::move(lock));
 }
 
+void FilesystemImpl::SetOpenedFileLength(base::File file,
+                                         uint64_t length,
+                                         SetOpenedFileLengthCallback callback) {
+  bool success = file.SetLength(length);
+  std::move(callback).Run(success, std::move(file));
+}
+
 // static
 FileErrorOr<base::File> FilesystemImpl::LockFileLocal(
     const base::FilePath& path) {
@@ -275,6 +287,29 @@ void FilesystemImpl::UnlockFileLocal(const base::FilePath& path) {
   GetLockTable().RemoveLock(path);
 }
 
+// static
+mojom::PathAccessInfoPtr FilesystemImpl::GetPathAccessLocal(
+    const base::FilePath& path) {
+  mojom::PathAccessInfoPtr info;
+#if defined(OS_WIN)
+  uint32_t attributes = ::GetFileAttributes(path.value().c_str());
+  if (attributes != INVALID_FILE_ATTRIBUTES) {
+    info = mojom::PathAccessInfo::New();
+    info->can_read = true;
+    if ((attributes & FILE_ATTRIBUTE_READONLY) == 0)
+      info->can_write = true;
+  }
+#elif defined(OS_POSIX) || defined(OS_FUCHSIA)
+  const char* const c_path = path.value().c_str();
+  if (!access(c_path, F_OK)) {
+    info = mojom::PathAccessInfo::New();
+    info->can_read = !access(c_path, R_OK);
+    info->can_write = !access(c_path, W_OK);
+  }
+#endif
+  return info;
+}
+
 // static
 FileErrorOr<std::vector<base::FilePath>> FilesystemImpl::GetDirectoryEntries(
     const base::FilePath& path,

components/services/storage/public/cpp/filesystem/filesystem_impl.h

@@ -58,16 +58,25 @@ class COMPONENT_EXPORT(STORAGE_SERVICE_FILESYSTEM_SUPPORT) FilesystemImpl
                        RemoveDirectoryCallback callback) override;
   void GetFileInfo(const base::FilePath& path,
                    GetFileInfoCallback callback) override;
+  void GetPathAccess(const base::FilePath& path,
+                     GetPathAccessCallback callback) override;
   void RenameFile(const base::FilePath& old_path,
                   const base::FilePath& new_path,
                   RenameFileCallback callback) override;
   void LockFile(const base::FilePath& path, LockFileCallback callback) override;
+  void SetOpenedFileLength(base::File file,
+                           uint64_t length,
+                           SetOpenedFileLengthCallback callback) override;
 
   // Helper used by LockFile() and FilesystemProxy::LockFile() for in
   // unrestricted mode.
   static FileErrorOr<base::File> LockFileLocal(const base::FilePath& path);
   static void UnlockFileLocal(const base::FilePath& path);
 
+  // Helper used by GetPathAccess() and FilesystemProxy::GetPathAccess.
+  static mojom::PathAccessInfoPtr GetPathAccessLocal(
+      const base::FilePath& path);
+
   // Helper used by GetEntries() and FilesystemProxy::GetDirectoryEntries.
   static FileErrorOr<std::vector<base::FilePath>> GetDirectoryEntries(
       const base::FilePath& path,

components/services/storage/public/cpp/filesystem/filesystem_proxy.cc

@@ -248,6 +248,20 @@ base::Optional<base::File::Info> FilesystemProxy::GetFileInfo(
   return info;
 }
 
+base::Optional<FilesystemProxy::PathAccessInfo> FilesystemProxy::GetPathAccess(
+    const base::FilePath& path) {
+  mojom::PathAccessInfoPtr info;
+  if (!remote_directory_)
+    info = FilesystemImpl::GetPathAccessLocal(MaybeMakeAbsolute(path));
+  else
+    remote_directory_->GetPathAccess(MakeRelative(path), &info);
+
+  if (!info)
+    return base::nullopt;
+
+  return PathAccessInfo{info->can_read, info->can_write};
+}
+
 base::File::Error FilesystemProxy::RenameFile(const base::FilePath& old_path,
                                               const base::FilePath& new_path) {
   base::File::Error error = base::File::FILE_ERROR_IO;
@@ -288,6 +302,16 @@ FilesystemProxy::LockFile(const base::FilePath& path) {
   return lock;
 }
 
+bool FilesystemProxy::SetOpenedFileLength(base::File* file, uint64_t length) {
+  if (!remote_directory_)
+    return file->SetLength(length);
+
+  bool success = false;
+  remote_directory_->SetOpenedFileLength(std::move(*file), length, &success,
+                                         file);
+  return success;
+}
+
 base::FilePath FilesystemProxy::MakeRelative(const base::FilePath& path) const {
   DCHECK(remote_directory_);
   DCHECK(!path.ReferencesParent());

components/services/storage/public/cpp/filesystem/filesystem_proxy.h

@@ -103,6 +103,14 @@ class COMPONENT_EXPORT(STORAGE_SERVICE_FILESYSTEM_SUPPORT) FilesystemProxy {
   // base::File::Info value on success, or null on failure.
   base::Optional<base::File::Info> GetFileInfo(const base::FilePath& path);
 
+  // Retrieves information about access rights for a path in the filesystem.
+  // Returns a valid PathAccessInfo on success, or null on failure.
+  struct PathAccessInfo {
+    bool can_read = false;
+    bool can_write = false;
+  };
+  base::Optional<PathAccessInfo> GetPathAccess(const base::FilePath& path);
+
   // Renames a file from |old_path| to |new_path|. Must be atomic.
   base::File::Error RenameFile(const base::FilePath& old_path,
                                const base::FilePath& new_path);
@@ -122,6 +130,9 @@ class COMPONENT_EXPORT(STORAGE_SERVICE_FILESYSTEM_SUPPORT) FilesystemProxy {
   };
   FileErrorOr<std::unique_ptr<FileLock>> LockFile(const base::FilePath& path);
 
+  // Sets the length of the given file to |length| bytes.
+  bool SetOpenedFileLength(base::File* file, uint64_t length);
+
  private:
   // For restricted FilesystemProxy instances, this returns a FilePath
   // equivalent to |path| which is strictly relative to |root_|. It is an error

components/services/storage/public/mojom/filesystem/directory.mojom

@@ -69,6 +69,16 @@ enum FileWriteAccess {
   kAppendOnly,
 };
 
+// A structure used to describe access rights for a specific path in the
+// filesystem.
+struct PathAccessInfo {
+  // Indicates whether the path can be opened for reading.
+  bool can_read;
+
+  // Indicates whether the path can be opened for writing.
+  bool can_write;
+};
+
 // An empty interface used to hold remote ownership of a lock acquired via
 // |Directory.LockFile()| below. Closing the remote endpoint of this interface
 // effectively releases the corresponding lock even if |Release()| is not
@@ -128,6 +138,11 @@ interface Directory {
   [Sync]
   GetFileInfo(StrictRelativePath path) => (mojo_base.mojom.FileInfo? info);
 
+  // Retrieves access information about a specific filesystem entry at |path|.
+  // Returns null on failure or a populated |info| struct on success.
+  [Sync]
+  GetPathAccess(StrictRelativePath path) => (PathAccessInfo? info);
+
   // Renames a file from |old_path| to |new_path|. Note that if |new_path|
   // already exists, it will be overwritten by this call.
   [Sync]
@@ -142,4 +157,19 @@ interface Directory {
   [Sync]
   LockFile(StrictRelativePath path) => (mojo_base.mojom.FileError error,
                                         pending_remote<FileLock>? lock);
+
+  // Asks the backend to set the length of the open file identified by |file|.
+  // |file| is returned back to the caller when done, regardless of success or
+  // failure.
+  //
+  // Note that this is a bit out-of-place on the Directory interface, but it's
+  // here out of an abundance of convenience and simplicity, to work around a
+  // (hopefully temporary) shortcoming of some sandbox configurations where
+  // enabling POSIX ftruncate() is not feasible.
+  //
+  // See https://crbug.com/1084565 for some history on this subject. In short,
+  // it's an issue with sandboxing on Mac OSX < 10.15.
+  [Sync]
+  SetOpenedFileLength(mojo_base.mojom.File file, uint64 size)
+      => (bool success, mojo_base.mojom.File file);
 };

components/services/storage/sandboxed_vfs_delegate.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/services/storage/sandboxed_vfs_delegate.h"
+
+#include <cstdint>
+#include <utility>
+
+#include "base/files/file.h"
+#include "base/files/file_path.h"
+#include "base/optional.h"
+#include "components/services/storage/public/cpp/filesystem/file_error_or.h"
+#include "components/services/storage/public/cpp/filesystem/filesystem_proxy.h"
+
+namespace storage {
+
+SandboxedVfsDelegate::SandboxedVfsDelegate(
+    std::unique_ptr<FilesystemProxy> filesystem)
+    : filesystem_(std::move(filesystem)) {}
+
+SandboxedVfsDelegate::~SandboxedVfsDelegate() = default;
+
+base::File SandboxedVfsDelegate::OpenFile(const base::FilePath& file_path,
+                                          int sqlite_requested_flags) {
+  FileErrorOr<base::File> result = filesystem_->OpenFile(
+      file_path, base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_READ |
+                     base::File::FLAG_WRITE);
+  if (result.is_error())
+    return base::File();
+  return std::move(result.value());
+}
+
+int SandboxedVfsDelegate::DeleteFile(const base::FilePath& file_path,
+                                     bool sync_dir) {
+  if (!filesystem_->RemoveFile(file_path))
+    return SQLITE_IOERR_DELETE;
+  return SQLITE_OK;
+}
+
+base::Optional<sql::SandboxedVfs::PathAccessInfo>
+SandboxedVfsDelegate::GetPathAccess(const base::FilePath& file_path) {
+  base::Optional<FilesystemProxy::PathAccessInfo> info =
+      filesystem_->GetPathAccess(file_path);
+  if (!info)
+    return base::nullopt;
+
+  sql::SandboxedVfs::PathAccessInfo access;
+  access.can_read = info->can_read;
+  access.can_write = info->can_write;
+  return access;
+}
+
+bool SandboxedVfsDelegate::SetFileLength(const base::FilePath& file_path,
+                                         base::File& file,
+                                         size_t size) {
+  return filesystem_->SetOpenedFileLength(&file, static_cast<uint64_t>(size));
+}
+
+}  // namespace storage

components/services/storage/sandboxed_vfs_delegate.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_SERVICES_STORAGE_SANDBOXED_VFS_DELEGATE_H_
+#define COMPONENTS_SERVICES_STORAGE_SANDBOXED_VFS_DELEGATE_H_
+
+#include <memory>
+
+#include "sql/sandboxed_vfs.h"
+
+namespace storage {
+
+class FilesystemProxy;
+
+class SandboxedVfsDelegate : public sql::SandboxedVfs::Delegate {
+ public:
+  explicit SandboxedVfsDelegate(std::unique_ptr<FilesystemProxy> filesystem);
+  ~SandboxedVfsDelegate() override;
+
+  // sql::SandboxedVfs::Delegate implementation:
+  base::File OpenFile(const base::FilePath& file_path,
+                      int sqlite_requested_flags) override;
+  base::Optional<sql::SandboxedVfs::PathAccessInfo> GetPathAccess(
+      const base::FilePath& file_path) override;
+  int DeleteFile(const base::FilePath& file_path, bool sync_dir) override;
+  bool SetFileLength(const base::FilePath& file_path,
+                     base::File& file,
+                     size_t size) override;
+
+ private:
+  const std::unique_ptr<FilesystemProxy> filesystem_;
+};
+
+}  // namespace storage
+
+#endif  // COMPONENTS_SERVICES_STORAGE_SANDBOXED_VFS_DELEGATE_H_

components/services/storage/storage_service_impl.cc

@@ -12,8 +12,11 @@
 #include "components/services/storage/filesystem_proxy_factory.h"
 #include "components/services/storage/partition_impl.h"
 #include "components/services/storage/public/cpp/filesystem/filesystem_proxy.h"
+#include "components/services/storage/sandboxed_vfs_delegate.h"
 #include "components/services/storage/test_api_stubs.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
+#include "sql/database.h"
+#include "sql/sandboxed_vfs.h"
 #include "third_party/leveldatabase/env_chromium.h"
 
 namespace storage {
@@ -23,6 +26,10 @@ namespace {
 // We don't use out-of-process Storage Service on Android, so we can avoid
 // pulling all the related code (including Directory mojom) into the build.
 #if !defined(OS_ANDROID)
+// The name under which we register our own sandboxed VFS instance when running
+// out-of-process.
+constexpr char kVfsName[] = "storage_service";
+
 using DirectoryBinder =
     base::RepeatingCallback<void(mojo::PendingReceiver<mojom::Directory>)>;
 std::unique_ptr<FilesystemProxy> CreateRestrictedFilesystemProxy(
@@ -71,6 +78,18 @@ void StorageServiceImpl::SetDataDirectory(
       base::BindRepeating(&StorageServiceImpl::BindDataDirectoryReceiver,
                           weak_ptr_factory_.GetWeakPtr()),
       base::SequencedTaskRunnerHandle::Get()));
+
+  // Prevent SQLite from trying to use mmap, as SandboxedVfs does not currently
+  // support this.
+  //
+  // TODO(crbug.com/1117049): Configure this per Database instance.
+  sql::Database::DisableMmapByDefault();
+
+  // SQLite needs our VFS implementation to work over a FilesystemProxy. This
+  // installs it as the default implementation for the service process.
+  sql::SandboxedVfs::Register(
+      kVfsName, std::make_unique<SandboxedVfsDelegate>(CreateFilesystemProxy()),
+      /*make_default=*/true);
 }
 #endif  // !defined(OS_ANDROID)
 

content/browser/dom_storage/dom_storage_browsertest.cc

@@ -12,6 +12,7 @@
 #include "components/services/storage/dom_storage/legacy_dom_storage_database.h"
 #include "components/services/storage/dom_storage/local_storage_impl.h"
 #include "components/services/storage/public/cpp/constants.h"
+#include "components/services/storage/public/cpp/filesystem/filesystem_proxy.h"
 #include "content/browser/dom_storage/dom_storage_context_wrapper.h"
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
@@ -174,7 +175,10 @@ IN_PROC_BROWSER_TEST_F(DOMStorageBrowserTest, DataMigrates) {
   {
     base::ScopedAllowBlockingForTesting allow_blocking;
     EXPECT_TRUE(base::CreateDirectory(legacy_local_storage_path));
-    storage::LegacyDomStorageDatabase db(db_path);
+    storage::LegacyDomStorageDatabase db(
+        db_path,
+        std::make_unique<storage::FilesystemProxy>(
+            storage::FilesystemProxy::UNRESTRICTED, legacy_local_storage_path));
     storage::LegacyDomStorageValuesMap data;
     data[base::ASCIIToUTF16("foo")] =
         base::NullableString16(base::ASCIIToUTF16("bar"), false);

sql/database.cc

@@ -40,6 +40,8 @@
 
 namespace {
 
+bool enable_mmap_by_default_ = true;
+
 // Spin for up to a second waiting for the lock to clear when setting
 // up the database.
 // TODO(shess): Better story on this.  http://crbug.com/56559
@@ -249,7 +251,7 @@ Database::Database()
       in_memory_(false),
       poisoned_(false),
       mmap_alt_status_(false),
-      mmap_disabled_(false),
+      mmap_disabled_(!enable_mmap_by_default_),
       mmap_enabled_(false),
       total_changes_at_last_release_(0),
       stats_histogram_(nullptr) {}
@@ -258,6 +260,10 @@ Database::~Database() {
   Close();
 }
 
+void Database::DisableMmapByDefault() {
+  enable_mmap_by_default_ = false;
+}
+
 void Database::RecordEvent(Events event, size_t count) {
   for (size_t i = 0; i < count; ++i) {
     UMA_HISTOGRAM_ENUMERATION("Sqlite.Stats2", event, EVENT_MAX_VALUE);
@@ -1006,11 +1012,11 @@ bool Database::Delete(const base::FilePath& path) {
   CHECK(vfs->xDelete);
   CHECK(vfs->xAccess);
 
-  // We only work with unix, win32 and mojo filesystems. If you're trying to
+  // We only work with the VFS implementations listed below. If you're trying to
   // use this code with any other VFS, you're not in a good place.
   CHECK(strncmp(vfs->zName, "unix", 4) == 0 ||
         strncmp(vfs->zName, "win32", 5) == 0 ||
-        strcmp(vfs->zName, "mojo") == 0);
+        strcmp(vfs->zName, "storage_service") == 0);
 
   vfs->xDelete(vfs, journal_str.c_str(), 0);
   vfs->xDelete(vfs, wal_str.c_str(), 0);

sql/database.h

@@ -60,6 +60,12 @@ class COMPONENT_EXPORT(SQL) Database {
   Database();
   ~Database();
 
+  // Allows mmapping to be disabled globally by default in the calling process.
+  // Must be called before any threads attempt to create a Database.
+  //
+  // TODO(crbug.com/1117049): Remove this global configuration.
+  static void DisableMmapByDefault();
+
   // Pre-init configuration ----------------------------------------------------
 
   // Sets the page size that will be used when creating a new database. This