dlp: Hook DLP ScopedFileAccess API into LocalFileStreamReader

As part of DLP file restrictions, the DLP daemon intercepts all file
access and by default blocks all access if there isn't a matching admin
rule that allows the op. There are two levels of DLP restrictions,
there is a high level check within the Files.app which already applies
file restrictions in the most part. The lower level DLP restrictions
within the //storage/ layer actually requests an access token for the
process to open the DLP restricted files.

As a first step, we will grant access to all requests in the lower DLP
layer by calling a DLP access request as a system component. This will
be switched out for a regular DLP access request method additionally
passing the destination target url when it can be obtained in //storage
(see crbug.com/1354502).

Change-Id: I53d9093280a8453e71faaa5b8a67627baf8308c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3858118
Reviewed-by: Aya Elsayed <[email protected]>
Reviewed-by: Austin Sullivan <[email protected]>
Commit-Queue: Alvin Lee <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1045745}

Author: Alvin Lee

chrome/browser/chromeos/policy/dlp/dlp_scoped_file_access_delegate.cc

@@ -6,6 +6,8 @@
 
 #include "base/process/process_handle.h"
 #include "chromeos/dbus/dlp/dlp_client.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 
 namespace policy {
 
@@ -68,9 +70,15 @@ void DlpScopedFileAccessDelegate::RequestFilesAccessForSystem(
 void DlpScopedFileAccessDelegate::PostRequestFileAccessToDaemon(
     const ::dlp::RequestFileAccessRequest request,
     base::OnceCallback<void(file_access::ScopedFileAccess)> callback) {
-  client_->RequestFileAccess(
-      request, base::BindOnce(&DlpScopedFileAccessDelegate::OnResponse,
-                              base::Unretained(this), std::move(callback)));
+  // base::Unretained is safe as |client_| (global dbus singleton) outlives the
+  // usage of |callback|.
+  auto dbus_cb = base::BindOnce(
+      &chromeos::DlpClient::RequestFileAccess, base::Unretained(client_),
+      request,
+      base::BindOnce(&DlpScopedFileAccessDelegate::OnResponse,
+                     base::Unretained(this), std::move(callback)));
+
+  content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, std::move(dbus_cb));
 }
 
 void DlpScopedFileAccessDelegate::OnResponse(
@@ -81,6 +89,7 @@ void DlpScopedFileAccessDelegate::OnResponse(
     std::move(callback).Run(file_access::ScopedFileAccess::Allowed());
     return;
   }
+
   std::move(callback).Run(
       file_access::ScopedFileAccess(response.allowed(), std::move(fd)));
 }

chrome/browser/chromeos/policy/dlp/dlp_scoped_file_access_delegate.h

@@ -61,6 +61,9 @@ class DlpScopedFileAccessDelegate
       const ::dlp::RequestFileAccessResponse response,
       base::ScopedFD fd);
 
+  // This is a pointer to a global dbus client singleton that is owned by the
+  // browser instance. It is initialized and destructed at the same time as
+  // dbus.
   raw_ptr<chromeos::DlpClient> client_;
 };
 

storage/DEPS

@@ -2,6 +2,7 @@ include_rules = [
   "+components/crash/core/common/crash_key.h",
   "+components/services/filesystem",
   "+components/services/storage",
+  "+components/file_access",
   "+crypto",
   "+mojo",
   "+net",

storage/browser/BUILD.gn

@@ -239,6 +239,7 @@ component("browser") {
     "//base/third_party/dynamic_annotations",
     "//build:chromeos_buildflags",
     "//components/crash/core/common:crash_key",
+    "//components/file_access:file_access",
     "//components/services/storage/public/cpp",
     "//components/services/storage/public/cpp/filesystem",
     "//mojo/public/cpp/bindings",
@@ -345,6 +346,7 @@ source_set("unittests") {
     ":test_support",
     "//base/test:test_support",
     "//build:chromeos_buildflags",
+    "//components/file_access:file_access",
     "//components/services/filesystem/public/mojom",
     "//components/services/storage/public/cpp",
     "//mojo/public/cpp/system",

storage/browser/file_system/local_file_stream_reader.cc

@@ -13,9 +13,12 @@
 #include "base/check_op.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
+#include "base/task/bind_post_task.h"
 #include "base/task/task_runner.h"
 #include "base/task/task_runner_util.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "base/types/pass_key.h"
+#include "components/file_access/scoped_file_access_delegate.h"
 #include "net/base/file_stream.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
@@ -93,16 +96,40 @@ void LocalFileStreamReader::Open(net::CompletionOnceCallback callback) {
   DCHECK(!stream_impl_.get());
   has_pending_open_ = true;
 
-  // Call GetLength first to make it perform last-modified-time verification,
-  // and then call DidVerifyForOpen to do the rest.
-  int64_t verify_result = GetLength(
-      base::BindOnce(&LocalFileStreamReader::DidVerifyForOpen,
-                     weak_factory_.GetWeakPtr(), std::move(callback)));
+  if (!file_access::ScopedFileAccessDelegate::Get()) {
+    OnScopedFileAccessRequested(std::move(callback),
+                                file_access::ScopedFileAccess::Allowed());
+    return;
+  }
+
+  base::OnceCallback<void(file_access::ScopedFileAccess)> open_cb =
+      base::BindOnce(&LocalFileStreamReader::OnScopedFileAccessRequested,
+                     weak_factory_.GetWeakPtr(), std::move(callback));
+  auto current_task_runner = base::SequencedTaskRunnerHandle::Get();
+  auto task = base::BindPostTask(current_task_runner, std::move(open_cb));
+
+  // TODO(crbug.com/1354502): Replace this with actual destination URLs.
+  file_access::ScopedFileAccessDelegate::Get()->RequestFilesAccessForSystem(
+      {file_path_}, std::move(task));
+}
+
+void LocalFileStreamReader::OnScopedFileAccessRequested(
+    net::CompletionOnceCallback callback,
+    file_access::ScopedFileAccess scoped_file_access) {
+  if (!scoped_file_access.is_allowed()) {
+    std::move(callback).Run(net::ERR_ACCESS_DENIED);
+    return;
+  }
+
+  int64_t verify_result = GetLength(base::BindOnce(
+      &LocalFileStreamReader::DidVerifyForOpen, weak_factory_.GetWeakPtr(),
+      std::move(callback), std::move(scoped_file_access)));
   DCHECK_EQ(verify_result, net::ERR_IO_PENDING);
 }
 
 void LocalFileStreamReader::DidVerifyForOpen(
     net::CompletionOnceCallback callback,
+    file_access::ScopedFileAccess scoped_file_access,
     int64_t get_length_result) {
   if (get_length_result < 0) {
     std::move(callback).Run(static_cast<int>(get_length_result));
@@ -114,12 +141,15 @@ void LocalFileStreamReader::DidVerifyForOpen(
   const int result = stream_impl_->Open(
       file_path_, kOpenFlagsForRead,
       base::BindOnce(&LocalFileStreamReader::DidOpenFileStream,
-                     weak_factory_.GetWeakPtr()));
+                     weak_factory_.GetWeakPtr(),
+                     std::move(scoped_file_access)));
   if (result != net::ERR_IO_PENDING)
     std::move(callback_).Run(result);
 }
 
-void LocalFileStreamReader::DidOpenFileStream(int result) {
+void LocalFileStreamReader::DidOpenFileStream(
+    file_access::ScopedFileAccess /*scoped_file_access*/,
+    int result) {
   if (result != net::OK) {
     std::move(callback_).Run(result);
     return;

storage/browser/file_system/local_file_stream_reader.h

@@ -15,6 +15,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "base/types/pass_key.h"
+#include "components/file_access/scoped_file_access.h"
 #include "net/base/completion_once_callback.h"
 #include "storage/browser/file_system/file_stream_reader.h"
 
@@ -50,9 +51,14 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) LocalFileStreamReader
   void Open(net::CompletionOnceCallback callback);
 
   // Callbacks that are chained from Open for Read.
+  void OnScopedFileAccessRequested(
+      net::CompletionOnceCallback callback,
+      file_access::ScopedFileAccess scoped_file_access);
   void DidVerifyForOpen(net::CompletionOnceCallback callback,
+                        file_access::ScopedFileAccess scoped_file_access,
                         int64_t get_length_result);
-  void DidOpenFileStream(int result);
+  void DidOpenFileStream(file_access::ScopedFileAccess /*scoped_file_access*/,
+                         int result);
   void DidSeekFileStream(int64_t seek_result);
   void DidOpenForRead(net::IOBuffer* buf,
                       int buf_len,

storage/browser/file_system/local_file_stream_reader_unittest.cc

@@ -22,15 +22,50 @@
 #include "base/test/task_environment.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
+#include "build/build_config.h"
+#include "components/file_access/scoped_file_access_delegate.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
 #include "storage/browser/file_system/file_stream_reader_test.h"
 #include "storage/browser/file_system/file_stream_test_utils.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
+#include "base/files/scoped_file.h"
+#endif
+
 namespace storage {
 
+namespace {
+
+using ::testing::_;
+
+class MockScopedFileAccessDelegate
+    : public file_access::ScopedFileAccessDelegate {
+ public:
+  MOCK_METHOD3(
+      RequestFilesAccess,
+      void(const std::vector<base::FilePath>& files,
+           const GURL& destination_url,
+           base::OnceCallback<void(file_access::ScopedFileAccess)> callback));
+  MOCK_METHOD2(
+      RequestFilesAccessForSystem,
+      void(const std::vector<base::FilePath>& files,
+           base::OnceCallback<void(file_access::ScopedFileAccess)> callback));
+};
+
+file_access::ScopedFileAccess CreateScopedFileAccess(bool allowed) {
+#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
+  return file_access::ScopedFileAccess(allowed, base::ScopedFD());
+#else
+  return file_access::ScopedFileAccess(allowed);
+#endif
+}
+
+}  // namespace
+
 class LocalFileStreamReaderTest : public FileStreamReaderTest {
  public:
   LocalFileStreamReaderTest() : file_thread_("TestFileThread") {}
@@ -100,4 +135,52 @@ INSTANTIATE_TYPED_TEST_SUITE_P(Local,
                                FileStreamReaderTypedTest,
                                LocalFileStreamReaderTest);
 
+// TODO(crbug.com/1354502): Use RequestFileAccess() instead of
+// RequestFileAccessForSystem() when destionion URLs can be obtained in
+// //storage/.
+TEST_F(LocalFileStreamReaderTest, ReadAllowedByDataLeakPrevention) {
+  this->WriteTestFile();
+  std::unique_ptr<FileStreamReader> reader(
+      this->CreateFileReader(std::string(this->kTestFileName), 0,
+                             this->test_file_modification_time()));
+
+  MockScopedFileAccessDelegate scoped_file_access_delegate;
+  EXPECT_CALL(scoped_file_access_delegate, RequestFilesAccessForSystem(_, _))
+      .WillOnce([&](const std::vector<base::FilePath>& files,
+                    base::OnceCallback<void(file_access::ScopedFileAccess)>
+                        callback) {
+        std::move(callback).Run(CreateScopedFileAccess(true));
+      });
+
+  int result = 0;
+  std::string data;
+  ReadFromReader(reader.get(), &data, this->kTestData.size(), &result);
+  ASSERT_EQ(net::OK, result);
+  ASSERT_EQ(this->kTestData, data);
+}
+
+// TODO(crbug.com/1354502): Use RequestFileAccess() instead of
+// RequestFileAccessForSystem() when destionion URLs can be obtained in
+// //storage/.
+TEST_F(LocalFileStreamReaderTest, ReadBlockedByDataLeakPrevention) {
+  this->WriteTestFile();
+  std::unique_ptr<FileStreamReader> reader(
+      this->CreateFileReader(std::string(this->kTestFileName), 0,
+                             this->test_file_modification_time()));
+
+  MockScopedFileAccessDelegate scoped_file_access_delegate;
+  EXPECT_CALL(scoped_file_access_delegate, RequestFilesAccessForSystem(_, _))
+      .WillOnce([&](const std::vector<base::FilePath>& files,
+                    base::OnceCallback<void(file_access::ScopedFileAccess)>
+                        callback) {
+        std::move(callback).Run(CreateScopedFileAccess(false));
+      });
+
+  int result = 0;
+  std::string data;
+  ReadFromReader(reader.get(), &data, this->kTestData.size(), &result);
+  ASSERT_EQ(net::ERR_ACCESS_DENIED, result);
+  ASSERT_EQ("", data);
+}
+
 }  // namespace storage