This is also very much related to issue 818327, where we currently have two completely different sets of checks when reading a File with FileReader and when uploading that same File using XHR/fetch. Ideally we would unify all those checks.

@mek: can you give me access to 818327?

Since we were just talking about sandboxing, probably a better solution instead of just doing a sync IPC to the browser to check a filepath is to have an IPC to exchange a path with a mojo file handle. The browser would only return the handle if the given process ID has access. That way the network service doesn't open the file in the network process, which will help with sandboxing.

jam:  You just send me a CL that sanboxes the network process.  If it can't do file IO, I assume this no longer matters - we'll have to refactor consumers not to send paths to the network service in the first place.

@Matt: Jay's cl just makes the URLLoaderFactory that's in the network process (and which is passed to renderers) not load file URLs. The issue in this bug still happens independently of that.

Penny has landed a cl (https://chromium-review.googlesource.com/c/chromium/src/+/1050703) that begins sandboxing, but that is off by default and will be off by default for a while (past canary).

 Issue 851663  has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee14fa1ea93c3ecdc389ac216cb9be2b266f714b

commit ee14fa1ea93c3ecdc389ac216cb9be2b266f714b
Author: Robbie McElrath <rmcelrath@chromium.org>
Date: Fri Jun 15 00:24:41 2018

Add NetworkServiceClient::OnFileUploadRequested method.

This is the start of a larger refactoring to the way file uploads are
handled when the network service is enabled. Currently a file path is
given to the network service, which then opens the file and uploads
its content. It doesn't currently check if the requester has access
to the file, and won't even be able to open files once it's sandboxed,
so the plan is to make the network service ask the browser process to
open the file for it. The renderer or browser process (whichever
wants to do the upload) will pass a file path to the network service,
which will then call a new method on NetworkServiceClient. This method
will take the file paths that it wants to upload, validate that the
originator of the request has permission to open those files (via
ChildProcessSecurityPolicy), open the files, and return the handles
back to the network service to upload them.

This CL defines just the new method, OnFileUploadRequested. Future CLs
will add its implementation.

Bug:  845612 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I9f7d0622db198a33e8f6dcdf9b8f4373d9156e2a
Reviewed-on: https://chromium-review.googlesource.com/1095653
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567497}
[modify] https://crrev.com/ee14fa1ea93c3ecdc389ac216cb9be2b266f714b/content/browser/network_service_client.cc
[modify] https://crrev.com/ee14fa1ea93c3ecdc389ac216cb9be2b266f714b/content/browser/network_service_client.h
[modify] https://crrev.com/ee14fa1ea93c3ecdc389ac216cb9be2b266f714b/services/network/network_service_unittest.cc
[modify] https://crrev.com/ee14fa1ea93c3ecdc389ac216cb9be2b266f714b/services/network/public/mojom/network_service.mojom
[modify] https://crrev.com/ee14fa1ea93c3ecdc389ac216cb9be2b266f714b/services/network/url_loader_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/addae866e40701808595648124c3332f32c23b52

commit addae866e40701808595648124c3332f32c23b52
Author: Robbie McElrath <rmcelrath@chromium.org>
Date: Tue Jun 19 19:40:35 2018

Make MessagePumpForIO::RegisterIOHandler return an error code

RegisterIOHandler calls CreateIoCompletionPort and will currently fail
silently. This adds a return code and propagates it up.

Bug:  845612 
Change-Id: Ie0685cb7fb6d21a5e23489337158ee0ba0b75154
Reviewed-on: https://chromium-review.googlesource.com/1101644
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568570}
[modify] https://crrev.com/addae866e40701808595648124c3332f32c23b52/base/message_loop/message_loop_current.cc
[modify] https://crrev.com/addae866e40701808595648124c3332f32c23b52/base/message_loop/message_loop_current.h
[modify] https://crrev.com/addae866e40701808595648124c3332f32c23b52/base/message_loop/message_pump_win.cc
[modify] https://crrev.com/addae866e40701808595648124c3332f32c23b52/base/message_loop/message_pump_win.h
[modify] https://crrev.com/addae866e40701808595648124c3332f32c23b52/net/base/file_stream_context_win.cc
[modify] https://crrev.com/addae866e40701808595648124c3332f32c23b52/net/base/file_stream_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d798079d315b6acedc797101a6c808ff826d32a0

commit d798079d315b6acedc797101a6c808ff826d32a0
Author: Robbie McElrath <rmcelrath@chromium.org>
Date: Wed Jun 20 00:24:46 2018

Add an async flag to mojo_base.mojo.File

Currently the async flag in base::File isn't serialized when passing
Files between processes. There are assertions and checks on the value
of this flag in several places, so it needs to be preserved during
serialization/deserialization. On Windows we can't check whether an
existing file handle is async or not, so we have to explicitly pass
the flag around.

Bug:  845612 
Change-Id: I8847ecd9451c13f7419e96db465caa417f4c543d
Reviewed-on: https://chromium-review.googlesource.com/1096480
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568662}
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/base/files/file.cc
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/base/files/file.h
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/base/files/file_posix.cc
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/base/files/file_unittest.cc
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/base/files/file_win.cc
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/chrome/browser/extensions/api/messaging/native_process_launcher_win.cc
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/mojo/public/cpp/base/file_mojom_traits.cc
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/mojo/public/cpp/base/file_mojom_traits.h
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/mojo/public/cpp/base/file_unittest.cc
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/mojo/public/mojom/base/file.mojom
[modify] https://crrev.com/d798079d315b6acedc797101a6c808ff826d32a0/net/base/file_stream_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ebcc06d2f1c78684c0c9929641005a00aefbfb6

commit 3ebcc06d2f1c78684c0c9929641005a00aefbfb6
Author: Robbie McElrath <rmcelrath@chromium.org>
Date: Thu Jun 28 00:53:09 2018

Implement and wire in NetworkServiceClient::OnFileUploadRequested.

This checks ChildProcessSecurityPolicy before opening the requested
files for upload.

Bug:  845612 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I16538e38bcb0a44acaf8c6dab4342dea8872f81f
Reviewed-on: https://chromium-review.googlesource.com/1098150
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570986}
[modify] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/chrome/browser/net/network_context_configuration_browsertest.cc
[modify] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/content/browser/network_service_client.cc
[modify] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/content/browser/network_service_client.h
[add] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/content/browser/network_service_client_unittest.cc
[modify] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/content/test/BUILD.gn
[modify] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/services/network/BUILD.gn
[modify] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/services/network/public/cpp/simple_url_loader_unittest.cc
[add] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/services/network/test/test_network_service_client.cc
[add] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/services/network/test/test_network_service_client.h
[modify] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/services/network/test/test_shared_url_loader_factory.cc
[modify] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/services/network/test/test_shared_url_loader_factory.h
[modify] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/services/network/url_loader.cc
[modify] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/services/network/url_loader.h
[modify] https://crrev.com/3ebcc06d2f1c78684c0c9929641005a00aefbfb6/services/network/url_loader_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3090048d25ddea91b9d725c4317a69b3808c2507

commit 3090048d25ddea91b9d725c4317a69b3808c2507
Author: Henrik Grunell <grunell@chromium.org>
Date: Thu Jun 28 08:03:02 2018

Revert "Implement and wire in NetworkServiceClient::OnFileUploadRequested."

This reverts commit 3ebcc06d2f1c78684c0c9929641005a00aefbfb6.

Reason for revert: Could be breaking unit_tests on Linux. It's the only suspicious CL I can find. https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests%20%28dbg%29%281%29%2832%29/50975

Original change's description:
> Implement and wire in NetworkServiceClient::OnFileUploadRequested.
> 
> This checks ChildProcessSecurityPolicy before opening the requested
> files for upload.
> 
> Bug:  845612 
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: I16538e38bcb0a44acaf8c6dab4342dea8872f81f
> Reviewed-on: https://chromium-review.googlesource.com/1098150
> Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570986}

TBR=jam@chromium.org,mmenke@chromium.org,rmcelrath@chromium.org

Change-Id: I00955100281cb36567b570e04742eece69367cf7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  845612 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1118118
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571045}
[modify] https://crrev.com/3090048d25ddea91b9d725c4317a69b3808c2507/chrome/browser/net/network_context_configuration_browsertest.cc
[modify] https://crrev.com/3090048d25ddea91b9d725c4317a69b3808c2507/content/browser/network_service_client.cc
[modify] https://crrev.com/3090048d25ddea91b9d725c4317a69b3808c2507/content/browser/network_service_client.h
[delete] https://crrev.com/2d5ab683ec764f238bf8571d950f90389051e320/content/browser/network_service_client_unittest.cc
[modify] https://crrev.com/3090048d25ddea91b9d725c4317a69b3808c2507/content/test/BUILD.gn
[modify] https://crrev.com/3090048d25ddea91b9d725c4317a69b3808c2507/services/network/BUILD.gn
[modify] https://crrev.com/3090048d25ddea91b9d725c4317a69b3808c2507/services/network/public/cpp/simple_url_loader_unittest.cc
[delete] https://crrev.com/2d5ab683ec764f238bf8571d950f90389051e320/services/network/test/test_network_service_client.cc
[delete] https://crrev.com/2d5ab683ec764f238bf8571d950f90389051e320/services/network/test/test_network_service_client.h
[modify] https://crrev.com/3090048d25ddea91b9d725c4317a69b3808c2507/services/network/test/test_shared_url_loader_factory.cc
[modify] https://crrev.com/3090048d25ddea91b9d725c4317a69b3808c2507/services/network/test/test_shared_url_loader_factory.h
[modify] https://crrev.com/3090048d25ddea91b9d725c4317a69b3808c2507/services/network/url_loader.cc
[modify] https://crrev.com/3090048d25ddea91b9d725c4317a69b3808c2507/services/network/url_loader.h
[modify] https://crrev.com/3090048d25ddea91b9d725c4317a69b3808c2507/services/network/url_loader_unittest.cc

I'm reopening this until I can reland it.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e11b28a0a627493301de48efd16906f19f990f5

commit 5e11b28a0a627493301de48efd16906f19f990f5
Author: Robbie McElrath <rmcelrath@chromium.org>
Date: Fri Jun 29 19:28:55 2018

Reland "Implement and wire in NetworkServiceClient::OnFileUploadRequested."

This is a reland of 3ebcc06d2f1c78684c0c9929641005a00aefbfb6

Original change's description:
> Implement and wire in NetworkServiceClient::OnFileUploadRequested.
>
> This checks ChildProcessSecurityPolicy before opening the requested
> files for upload.
>
> Bug:  845612 
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: I16538e38bcb0a44acaf8c6dab4342dea8872f81f
> Reviewed-on: https://chromium-review.googlesource.com/1098150
> Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570986}

Bug:  845612 
Change-Id: I47a0ad37a7e2e219aa68d11e8c3f141029496dc3
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1118786
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571589}
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/chrome/browser/net/network_context_configuration_browsertest.cc
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/chrome/browser/safe_browsing/download_protection/two_phase_uploader_unittest.cc
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/content/browser/network_service_client.cc
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/content/browser/network_service_client.h
[add] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/content/browser/network_service_client_unittest.cc
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/content/test/BUILD.gn
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/services/network/BUILD.gn
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/services/network/public/cpp/simple_url_loader_unittest.cc
[add] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/services/network/test/test_network_service_client.cc
[add] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/services/network/test/test_network_service_client.h
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/services/network/test/test_shared_url_loader_factory.cc
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/services/network/test/test_shared_url_loader_factory.h
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/services/network/url_loader.cc
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/services/network/url_loader.h
[modify] https://crrev.com/5e11b28a0a627493301de48efd16906f19f990f5/services/network/url_loader_unittest.cc

 Issue 777879  has been merged into this issue.