Adds mojo_base.mojom.ReadOnlyFile

Adds a mojo ReadOnlyFile that won't serialize if it can be written.

The expectation is that this handle will then be sent to another process
which lacks the rights to add any write permissions to the handle. This
may not be true on all platforms but is useful where this holds.

Code to check for readonly-ness is implemented within mojo rather than
base::File as it is (a) difficult to say in general what it means to be
readonly, and (b) not possible on every platform.

Bug: 1130762
Change-Id: I6f00934005da7e433af4fee94454dfcd28dfd3c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2417549
Commit-Queue: Alex Gough <ajgo@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813987}

mojo/public/cpp/base/BUILD.gn

@@ -90,6 +90,8 @@ component("shared_typemap_traits") {
     "generic_pending_receiver_mojom_traits.h",
     "read_only_buffer_mojom_traits.cc",
     "read_only_buffer_mojom_traits.h",
+    "read_only_file_mojom_traits.cc",
+    "read_only_file_mojom_traits.h",
     "shared_memory_mojom_traits.cc",
     "shared_memory_mojom_traits.h",
     "time_mojom_traits.cc",

mojo/public/cpp/base/file_unittest.cc

@@ -3,9 +3,13 @@
 // found in the LICENSE file.
 
 #include "base/files/scoped_temp_dir.h"
+#include "base/sync_socket.h"
+#include "build/build_config.h"
 #include "mojo/public/cpp/base/file_mojom_traits.h"
+#include "mojo/public/cpp/base/read_only_file_mojom_traits.h"
 #include "mojo/public/cpp/test_support/test_utils.h"
 #include "mojo/public/mojom/base/file.mojom.h"
+#include "mojo/public/mojom/base/read_only_file.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace mojo_base {
@@ -69,5 +73,83 @@ TEST(FileTest, InvalidFile) {
   EXPECT_FALSE(file_out.IsValid());
 }
 
+TEST(FileTest, ReadOnlyFile) {
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  base::File file(
+      temp_dir.GetPath().AppendASCII("test_file.txt"),
+      base::File::FLAG_CREATE | base::File::FLAG_WRITE | base::File::FLAG_READ);
+  const base::StringPiece test_content =
+      "A test string to be stored in a test file";
+  file.WriteAtCurrentPos(test_content.data(),
+                         base::checked_cast<int>(test_content.size()));
+  file.Close();
+
+  base::File readonly(temp_dir.GetPath().AppendASCII("test_file.txt"),
+                      base::File::FLAG_OPEN | base::File::FLAG_READ);
+
+  base::File file_out;
+  ASSERT_TRUE(mojo::test::SerializeAndDeserialize<mojom::ReadOnlyFile>(
+      &readonly, &file_out));
+  std::vector<char> content(test_content.size());
+  ASSERT_TRUE(file_out.IsValid());
+  ASSERT_FALSE(file_out.async());
+  ASSERT_EQ(static_cast<int>(test_content.size()),
+            file_out.Read(0, content.data(),
+                          base::checked_cast<int>(test_content.size())));
+  EXPECT_EQ(test_content,
+            base::StringPiece(content.data(), test_content.size()));
+}
+
+// This dies only if we can interrogate the underlying platform handle.
+#if defined(OS_WIN) || defined(OS_POSIX) || defined(OS_FUCHSIA)
+#if !defined(OS_NACL) && !defined(OS_AIX)
+TEST(FileTest, ReadOnlyFileDeath) {
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  base::File file(
+      temp_dir.GetPath().AppendASCII("test_file.txt"),
+      base::File::FLAG_CREATE | base::File::FLAG_WRITE | base::File::FLAG_READ);
+  const base::StringPiece test_content =
+      "A test string to be stored in a test file";
+  file.WriteAtCurrentPos(test_content.data(),
+                         base::checked_cast<int>(test_content.size()));
+  file.Close();
+
+  base::File writable(
+      temp_dir.GetPath().AppendASCII("test_file.txt"),
+      base::File::FLAG_OPEN | base::File::FLAG_READ | base::File::FLAG_WRITE);
+
+  base::File file_out;
+  EXPECT_DEATH_IF_SUPPORTED(
+      mojo::test::SerializeAndDeserialize<mojom::ReadOnlyFile>(&writable,
+                                                               &file_out),
+      "Check failed: IsReadOnlyFile");
+}
+#endif  // !defined(OS_NACL) && !defined(OS_AIX)
+#endif  // defined(OS_WIN) || defined(OS_POSIX) || defined(OS_FUCHSIA)
+
+// This should work on all platforms. This check might be relaxed in which case
+// this test can be removed.
+TEST(FileTest, NonPhysicalFileDeath) {
+  base::SyncSocket sync_a;
+  base::SyncSocket sync_b;
+  ASSERT_TRUE(base::SyncSocket::CreatePair(&sync_a, &sync_b));
+  base::File file_pipe_a(sync_a.Take());
+  base::File file_pipe_b(sync_b.Take());
+
+  base::File file_out;
+  EXPECT_DEATH_IF_SUPPORTED(
+      mojo::test::SerializeAndDeserialize<mojom::ReadOnlyFile>(&file_pipe_a,
+                                                               &file_out),
+      "Check failed: IsPhysicalFile");
+  EXPECT_DEATH_IF_SUPPORTED(
+      mojo::test::SerializeAndDeserialize<mojom::ReadOnlyFile>(&file_pipe_b,
+                                                               &file_out),
+      "Check failed: IsPhysicalFile");
+}
+
 }  // namespace file_unittest
 }  // namespace mojo_base

mojo/public/cpp/base/read_only_file_mojom_traits.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 "mojo/public/cpp/base/read_only_file_mojom_traits.h"
+
+#include "base/files/file.h"
+#include "build/build_config.h"
+
+#if defined(OS_POSIX) || defined(OS_FUCHSIA)
+#include <fcntl.h>
+#endif
+#if defined(OS_WIN)
+#include <windows.h>
+#include <winternl.h>
+#endif
+
+namespace mojo {
+namespace {
+#if defined(OS_WIN)
+bool GetGrantedAccess(HANDLE handle, DWORD* flags) {
+  static const auto nt_query_object =
+      reinterpret_cast<decltype(&NtQueryObject)>(
+          GetProcAddress(GetModuleHandle(L"ntdll.dll"), "NtQueryObject"));
+  PUBLIC_OBJECT_BASIC_INFORMATION info;
+  ULONG len = sizeof(info);
+  ULONG consumed = 0;
+  auto ret =
+      nt_query_object(handle, ObjectBasicInformation, &info, len, &consumed);
+  if (ret)
+    return false;
+  *flags = info.GrantedAccess;
+  return true;
+}
+#endif  // defined(OS_WIN)
+
+// True if the underlying handle is only readable. Where possible this excludes
+// deletion, writing, truncation, append and other operations that might modify
+// the underlying file. False if we can tell that the file could be modified.
+// On platforms where we cannot test the handle, always returns true.
+bool IsReadOnlyFile(base::File& file) {
+  bool is_readonly = true;
+#if defined(OS_WIN)
+  DWORD flags = 0;
+  if (!GetGrantedAccess(file.GetPlatformFile(), &flags))
+    return false;
+
+  // Cannot use GENERIC_WRITE as that includes SYNCHRONIZE.
+  // This is ~(all the writable permissions).
+  is_readonly =
+      !(flags & (FILE_APPEND_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_DATA |
+                 FILE_WRITE_EA | WRITE_DAC | WRITE_OWNER | DELETE));
+#elif defined(OS_FUCHSIA) || \
+    (defined(OS_POSIX) && !defined(OS_NACL) && !defined(OS_AIX))
+  is_readonly =
+      (fcntl(file.GetPlatformFile(), F_GETFL) & O_ACCMODE) == O_RDONLY;
+#endif
+  return is_readonly;
+}
+
+bool IsPhysicalFile(base::File& file) {
+#if defined(OS_WIN)
+  // Verify if this is a real file (not a socket/pipe etc.).
+  DWORD type = GetFileType(file.GetPlatformFile());
+  return type == FILE_TYPE_DISK;
+#else
+  base::stat_wrapper_t stat;
+  if (base::File::Fstat(file.GetPlatformFile(), &stat) != 0)
+    return false;
+  return S_ISREG(stat.st_mode);
+#endif
+}
+
+}  // namespace
+
+mojo::PlatformHandle StructTraits<mojo_base::mojom::ReadOnlyFileDataView,
+                                  base::File>::fd(base::File& file) {
+  DCHECK(file.IsValid());
+  // For now we require real files as on some platforms it is too difficult to
+  // be sure that more general handles cannot be written or made writable. This
+  // could be relaxed if an interface needs readonly pipes.
+  CHECK(IsPhysicalFile(file));
+  CHECK(IsReadOnlyFile(file));
+
+  return mojo::PlatformHandle(
+      base::ScopedPlatformFile(file.TakePlatformFile()));
+}
+
+bool StructTraits<mojo_base::mojom::ReadOnlyFileDataView, base::File>::Read(
+    mojo_base::mojom::ReadOnlyFileDataView data,
+    base::File* file) {
+  *file = base::File(data.TakeFd().TakePlatformFile(), data.async());
+  return true;
+}
+
+}  // namespace mojo

mojo/public/cpp/base/read_only_file_mojom_traits.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 MOJO_PUBLIC_CPP_BASE_READ_ONLY_FILE_MOJOM_TRAITS_H_
+#define MOJO_PUBLIC_CPP_BASE_READ_ONLY_FILE_MOJOM_TRAITS_H_
+
+#include "base/component_export.h"
+#include "base/files/file.h"
+#include "mojo/public/mojom/base/read_only_file.mojom-shared.h"
+
+namespace mojo {
+
+template <>
+struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS)
+    StructTraits<mojo_base::mojom::ReadOnlyFileDataView, base::File> {
+  static bool IsNull(const base::File& file) { return !file.IsValid(); }
+
+  static void SetToNull(base::File* file) { *file = base::File(); }
+
+  static mojo::PlatformHandle fd(base::File& file);
+  static bool async(base::File& file) { return file.async(); }
+  static bool Read(mojo_base::mojom::ReadOnlyFileDataView data,
+                   base::File* file);
+};
+
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_CPP_BASE_READ_ONLY_FILE_MOJOM_TRAITS_H_

mojo/public/mojom/base/BUILD.gn

@@ -21,6 +21,7 @@ mojom_component("base") {
     "message_pump_type.mojom",
     "process_id.mojom",
     "read_only_buffer.mojom",
+    "read_only_file.mojom",
     "ref_counted_memory.mojom",
     "shared_memory.mojom",
     "string16.mojom",
@@ -118,6 +119,24 @@ mojom_component("base") {
         "//mojo/public/cpp/base:shared_typemap_traits",
       ]
     },
+    {
+      types = [
+        {
+          mojom = "mojo_base.mojom.ReadOnlyFile"
+          cpp = "::base::File"
+          move_only = true
+          nullable_is_same_type = true
+          force_serialize = true
+        },
+      ]
+      traits_headers =
+          [ "//mojo/public/cpp/base/read_only_file_mojom_traits.h" ]
+      traits_public_deps = [
+        "//base",
+        "//mojo/public/cpp/base",
+        "//mojo/public/cpp/base:shared_typemap_traits",
+      ]
+    },
     {
       types = [
         {

mojo/public/mojom/base/read_only_file.mojom

+// 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.
+
+module mojo_base.mojom;
+
+// Corresponds to |base::File| in base/files/file.h but, on most
+// platforms, will not serialise handles which are writable. At
+// present this only supports physically backed files, but this may be
+// relaxed in future.
+//
+// SECURITY_NOTE: This type is an indication that a readonly handle can
+// be provided. A sandboxed process should ensure that the handle cannot
+// be made writable. This may not be possible on all platforms.
+//
+// See |file_mojom_traits.cc| for details.
+struct ReadOnlyFile {
+  handle<platform> fd;
+  bool async;
+};