Sandbox the component updater's patcher utility process.

The code will now open files in the browser process before passing the
handles across IPC to the utility process. The utility process in turn
invokes courgette/bsdiff, which memory maps the files and operates on
them as before.

There is a behavioral difference when using the courgette or
courgette_mini tools: the output file will now be created/overwritten
at the start of the operation, and in the case of a failure, will be
deleted. Previously, the output file was created late in the operation
operation and several failure modes would leave it unmodified.

BUG=660325

Review-Url: https://codereview.chromium.org/2534873005
Cr-Commit-Position: refs/heads/master@{#437669}

chrome/browser/component_updater/component_patcher_operation_out_of_process.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/component_updater/component_patcher_operation_out_of_process.h"
 
+#include <memory>
 #include <vector>
 
 #include "base/bind.h"
@@ -61,7 +62,6 @@ void PatchHost::StartProcess(std::unique_ptr<IPC::Message> message) {
       this, base::ThreadTaskRunnerHandle::Get().get());
   host->SetName(l10n_util::GetStringUTF16(
       IDS_UTILITY_PROCESS_COMPONENT_PATCHER_NAME));
-  host->DisableSandbox();
   host->Send(message.release());
 }
 
@@ -102,13 +102,25 @@ void ChromeOutOfProcessPatcher::Patch(
     const base::FilePath& output_abs_path,
     base::Callback<void(int result)> callback) {
   host_ = new PatchHost(callback, task_runner);
+  IPC::PlatformFileForTransit input = IPC::TakePlatformFileForTransit(
+      base::File(
+          input_abs_path, base::File::FLAG_OPEN | base::File::FLAG_READ));
+  IPC::PlatformFileForTransit patch = IPC::TakePlatformFileForTransit(
+      base::File(
+          patch_abs_path, base::File::FLAG_OPEN | base::File::FLAG_READ));
+  IPC::PlatformFileForTransit output = IPC::TakePlatformFileForTransit(
+      base::File(
+          output_abs_path,
+          base::File::FLAG_CREATE |
+              base::File::FLAG_WRITE |
+              base::File::FLAG_EXCLUSIVE_WRITE));
   std::unique_ptr<IPC::Message> patch_message;
   if (operation == update_client::kBsdiff) {
     patch_message.reset(new ChromeUtilityMsg_PatchFileBsdiff(
-        input_abs_path, patch_abs_path, output_abs_path));
+        input, patch, output));
   } else if (operation == update_client::kCourgette) {
     patch_message.reset(new ChromeUtilityMsg_PatchFileCourgette(
-        input_abs_path, patch_abs_path, output_abs_path));
+        input, patch, output));
   } else {
     NOTREACHED();
   }

chrome/common/chrome_utility_messages.h

@@ -144,17 +144,17 @@ IPC_STRUCT_END()
 // and place the output in |output_file|. The patch should use the bsdiff
 // algorithm (Courgette's version).
 IPC_MESSAGE_CONTROL3(ChromeUtilityMsg_PatchFileBsdiff,
-                     base::FilePath /* input_file */,
-                     base::FilePath /* patch_file */,
-                     base::FilePath /* output_file */)
+                     IPC::PlatformFileForTransit /* input_file */,
+                     IPC::PlatformFileForTransit /* patch_file */,
+                     IPC::PlatformFileForTransit /* output_file */)
 
 // Tell the utility process to patch the given |input_file| using |patch_file|
 // and place the output in |output_file|. The patch should use the Courgette
 // algorithm.
 IPC_MESSAGE_CONTROL3(ChromeUtilityMsg_PatchFileCourgette,
-                     base::FilePath /* input_file */,
-                     base::FilePath /* patch_file */,
-                     base::FilePath /* output_file */)
+                     IPC::PlatformFileForTransit /* input_file */,
+                     IPC::PlatformFileForTransit /* patch_file */,
+                     IPC::PlatformFileForTransit /* output_file */)
 
 #if defined(OS_CHROMEOS)
 // Tell the utility process to create a zip file on the given list of files.

chrome/utility/chrome_content_utility_client.cc

@@ -265,33 +265,26 @@ void ChromeContentUtilityClient::OnCreateZipFile(
 #endif  // defined(OS_CHROMEOS)
 
 void ChromeContentUtilityClient::OnPatchFileBsdiff(
-    const base::FilePath& input_file,
-    const base::FilePath& patch_file,
-    const base::FilePath& output_file) {
-  if (input_file.empty() || patch_file.empty() || output_file.empty()) {
-    Send(new ChromeUtilityHostMsg_PatchFile_Finished(-1));
-  } else {
-    const int patch_status = bsdiff::ApplyBinaryPatch(input_file,
-                                                      patch_file,
-                                                      output_file);
-    Send(new ChromeUtilityHostMsg_PatchFile_Finished(patch_status));
-  }
+    const IPC::PlatformFileForTransit& input_file,
+    const IPC::PlatformFileForTransit& patch_file,
+    const IPC::PlatformFileForTransit& output_file) {
+  const int patch_status = bsdiff::ApplyBinaryPatch(
+      IPC::PlatformFileForTransitToFile(input_file),
+      IPC::PlatformFileForTransitToFile(patch_file),
+      IPC::PlatformFileForTransitToFile(output_file));
+  Send(new ChromeUtilityHostMsg_PatchFile_Finished(patch_status));
   ReleaseProcessIfNeeded();
 }
 
 void ChromeContentUtilityClient::OnPatchFileCourgette(
-    const base::FilePath& input_file,
-    const base::FilePath& patch_file,
-    const base::FilePath& output_file) {
-  if (input_file.empty() || patch_file.empty() || output_file.empty()) {
-    Send(new ChromeUtilityHostMsg_PatchFile_Finished(-1));
-  } else {
-    const int patch_status = courgette::ApplyEnsemblePatch(
-        input_file.value().c_str(),
-        patch_file.value().c_str(),
-        output_file.value().c_str());
-    Send(new ChromeUtilityHostMsg_PatchFile_Finished(patch_status));
-  }
+    const IPC::PlatformFileForTransit& input_file,
+    const IPC::PlatformFileForTransit& patch_file,
+    const IPC::PlatformFileForTransit& output_file) {
+  const int patch_status = courgette::ApplyEnsemblePatch(
+      IPC::PlatformFileForTransitToFile(input_file),
+      IPC::PlatformFileForTransitToFile(patch_file),
+      IPC::PlatformFileForTransitToFile(output_file));
+  Send(new ChromeUtilityHostMsg_PatchFile_Finished(patch_status));
   ReleaseProcessIfNeeded();
 }
 

chrome/utility/chrome_content_utility_client.h

@@ -50,12 +50,12 @@ class ChromeContentUtilityClient : public content::ContentUtilityClient {
                        const base::FileDescriptor& dest_fd);
 #endif  // defined(OS_CHROMEOS)
 
-  void OnPatchFileBsdiff(const base::FilePath& input_file,
-                         const base::FilePath& patch_file,
-                         const base::FilePath& output_file);
-  void OnPatchFileCourgette(const base::FilePath& input_file,
-                            const base::FilePath& patch_file,
-                            const base::FilePath& output_file);
+  void OnPatchFileBsdiff(const IPC::PlatformFileForTransit& input_file,
+                         const IPC::PlatformFileForTransit& patch_file,
+                         const IPC::PlatformFileForTransit& output_file);
+  void OnPatchFileCourgette(const IPC::PlatformFileForTransit& input_file,
+                            const IPC::PlatformFileForTransit& patch_file,
+                            const IPC::PlatformFileForTransit& output_file);
   void OnStartupPing();
 #if defined(FULL_SAFE_BROWSING)
   void OnAnalyzeZipFileForDownloadProtection(

courgette/courgette.h

@@ -7,6 +7,7 @@
 
 #include <stddef.h>   // Required to define size_t on GCC
 
+#include "base/files/file.h"
 #include "base/files/file_path.h"
 
 namespace courgette {
@@ -73,6 +74,15 @@ class EncodedProgram;
 Status ApplyEnsemblePatch(SourceStream* old, SourceStream* patch,
                           SinkStream* output);
 
+// Applies the patch in |patch_file| to the bytes in |old_file| and writes the
+// transformed ensemble to |new_file|.
+// Returns C_OK unless something went wrong.
+// This function first validates that the patch file has a proper header, so the
+// function can be used to 'try' a patch.
+Status ApplyEnsemblePatch(base::File old_file,
+                          base::File patch_file,
+                          base::File new_file);
+
 // Applies the patch in |patch_file_name| to the bytes in |old_file_name| and
 // writes the transformed ensemble to |new_file_name|.
 // Returns C_OK unless something went wrong.

courgette/ensemble_apply.cc

@@ -12,6 +12,7 @@
 #include <memory>
 #include <utility>
 
+#include "base/files/file.h"
 #include "base/files/file_util.h"
 #include "base/files/memory_mapped_file.h"
 #include "base/logging.h"
@@ -371,46 +372,42 @@ Status ApplyEnsemblePatch(SourceStream* base,
   return C_OK;
 }
 
-Status ApplyEnsemblePatch(const base::FilePath::CharType* old_file_name,
-                          const base::FilePath::CharType* patch_file_name,
-                          const base::FilePath::CharType* new_file_name) {
-  base::FilePath patch_file_path(patch_file_name);
-  base::MemoryMappedFile patch_file;
-  if (!patch_file.Initialize(patch_file_path))
+Status ApplyEnsemblePatch(base::File old_file,
+                          base::File patch_file,
+                          base::File new_file) {
+  base::MemoryMappedFile patch_file_mem;
+  if (!patch_file_mem.Initialize(std::move(patch_file)))
     return C_READ_OPEN_ERROR;
 
   // 'Dry-run' the first step of the patch process to validate format of header.
   SourceStream patch_header_stream;
-  patch_header_stream.Init(patch_file.data(), patch_file.length());
+  patch_header_stream.Init(patch_file_mem.data(), patch_file_mem.length());
   EnsemblePatchApplication patch_process;
   Status status = patch_process.ReadHeader(&patch_header_stream);
   if (status != C_OK)
     return status;
 
   // Read the old_file.
-  base::FilePath old_file_path(old_file_name);
-  base::MemoryMappedFile old_file;
-  if (!old_file.Initialize(old_file_path))
+  base::MemoryMappedFile old_file_mem;
+  if (!old_file_mem.Initialize(std::move(old_file)))
     return C_READ_ERROR;
 
   // Apply patch on streams.
   SourceStream old_source_stream;
   SourceStream patch_source_stream;
-  old_source_stream.Init(old_file.data(), old_file.length());
-  patch_source_stream.Init(patch_file.data(), patch_file.length());
+  old_source_stream.Init(old_file_mem.data(), old_file_mem.length());
+  patch_source_stream.Init(patch_file_mem.data(), patch_file_mem.length());
   SinkStream new_sink_stream;
   status = ApplyEnsemblePatch(&old_source_stream, &patch_source_stream,
                               &new_sink_stream);
   if (status != C_OK)
     return status;
 
   // Write the patched data to |new_file_name|.
-  base::FilePath new_file_path(new_file_name);
-  int written =
-      base::WriteFile(
-          new_file_path,
-          reinterpret_cast<const char*>(new_sink_stream.Buffer()),
-          static_cast<int>(new_sink_stream.Length()));
+  int written = new_file.Write(
+      0,
+      reinterpret_cast<const char*>(new_sink_stream.Buffer()),
+      static_cast<int>(new_sink_stream.Length()));
   if (written == -1)
     return C_WRITE_OPEN_ERROR;
   if (static_cast<size_t>(written) != new_sink_stream.Length())
@@ -419,4 +416,24 @@ Status ApplyEnsemblePatch(const base::FilePath::CharType* old_file_name,
   return C_OK;
 }
 
-}  // namespace
+Status ApplyEnsemblePatch(const base::FilePath::CharType* old_file_name,
+                          const base::FilePath::CharType* patch_file_name,
+                          const base::FilePath::CharType* new_file_name) {
+  Status result = ApplyEnsemblePatch(
+      base::File(
+          base::FilePath(old_file_name),
+          base::File::FLAG_OPEN | base::File::FLAG_READ),
+      base::File(
+          base::FilePath(patch_file_name),
+          base::File::FLAG_OPEN | base::File::FLAG_READ),
+      base::File(
+          base::FilePath(new_file_name),
+          base::File::FLAG_CREATE_ALWAYS |
+              base::File::FLAG_WRITE |
+              base::File::FLAG_EXCLUSIVE_WRITE));
+  if (result != C_OK)
+    base::DeleteFile(base::FilePath(new_file_name), false);
+  return result;
+}
+
+}  // namespace courgette

courgette/third_party/bsdiff/bsdiff.h

@@ -44,6 +44,7 @@
 
 #include <stdint.h>
 
+#include "base/files/file.h"
 #include "base/files/file_util.h"
 
 namespace courgette {
@@ -76,6 +77,11 @@ BSDiffStatus ApplyBinaryPatch(courgette::SourceStream* old_stream,
                               courgette::SourceStream* patch_stream,
                               courgette::SinkStream* new_stream);
 
+// As above, but simply takes base::Files.
+BSDiffStatus ApplyBinaryPatch(base::File old_stream,
+                              base::File patch_stream,
+                              base::File new_stream);
+
 // As above, but simply takes the file paths.
 BSDiffStatus ApplyBinaryPatch(const base::FilePath& old_stream,
                               const base::FilePath& patch_stream,

courgette/third_party/bsdiff/bsdiff_apply.cc

@@ -40,6 +40,7 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include "base/files/file_util.h"
 #include "base/files/memory_mapped_file.h"
 #include "courgette/crc.h"
 #include "courgette/streams.h"
@@ -189,24 +190,24 @@ BSDiffStatus ApplyBinaryPatch(SourceStream* old_stream,
   return OK;
 }
 
-BSDiffStatus ApplyBinaryPatch(const base::FilePath& old_file_path,
-                              const base::FilePath& patch_file_path,
-                              const base::FilePath& new_file_path) {
+BSDiffStatus ApplyBinaryPatch(base::File old_file,
+                              base::File patch_file,
+                              base::File new_file) {
   // Set up the old stream.
-  base::MemoryMappedFile old_file;
-  if (!old_file.Initialize(old_file_path)) {
+  base::MemoryMappedFile old_file_mem;
+  if (!old_file_mem.Initialize(std::move(old_file))) {
     return READ_ERROR;
   }
   SourceStream old_file_stream;
-  old_file_stream.Init(old_file.data(), old_file.length());
+  old_file_stream.Init(old_file_mem.data(), old_file_mem.length());
 
   // Set up the patch stream.
-  base::MemoryMappedFile patch_file;
-  if (!patch_file.Initialize(patch_file_path)) {
+  base::MemoryMappedFile patch_file_mem;
+  if (!patch_file_mem.Initialize(std::move(patch_file))) {
     return READ_ERROR;
   }
   SourceStream patch_file_stream;
-  patch_file_stream.Init(patch_file.data(), patch_file.length());
+  patch_file_stream.Init(patch_file_mem.data(), patch_file_mem.length());
 
   // Set up the new stream and apply the patch.
   SinkStream new_sink_stream;
@@ -217,12 +218,33 @@ BSDiffStatus ApplyBinaryPatch(const base::FilePath& old_file_path,
   }
 
   // Write the stream to disk.
-  int written = base::WriteFile(
-      new_file_path, reinterpret_cast<const char*>(new_sink_stream.Buffer()),
+  int written = new_file.Write(
+      0,
+      reinterpret_cast<const char*>(new_sink_stream.Buffer()),
       static_cast<int>(new_sink_stream.Length()));
   if (written != static_cast<int>(new_sink_stream.Length()))
     return WRITE_ERROR;
   return OK;
 }
 
+BSDiffStatus ApplyBinaryPatch(const base::FilePath& old_file_path,
+                              const base::FilePath& patch_file_path,
+                              const base::FilePath& new_file_path) {
+  BSDiffStatus result = ApplyBinaryPatch(
+      base::File(
+          old_file_path,
+          base::File::FLAG_OPEN | base::File::FLAG_READ),
+      base::File(
+          patch_file_path,
+          base::File::FLAG_OPEN | base::File::FLAG_READ),
+      base::File(
+          new_file_path,
+          base::File::FLAG_CREATE_ALWAYS |
+              base::File::FLAG_WRITE |
+              base::File::FLAG_EXCLUSIVE_WRITE));
+  if (result != OK)
+    base::DeleteFile(new_file_path, false);
+  return result;
+}
+
 }  // namespace bsdiff