i don't think we should allow writing through symlinks at all in removable media.  your mount patch from a while ago would help here ;).

Regarding the mount patch: I'm still aiming to complete the quest of stopping symlink traversal. I have a pending kernel CL that implements this in the chromiumos LSM: https://chromium-review.googlesource.com/#/c/420988/

Once this is landed on all kernels, I intend to gradually lock down stateful, and we can also use this to block symlink traversal on removable media.

On a related note, we might consider doing something smarter, i.e. not outright block symlink traversal, but only block symlinks that point at a different file system. It should be possible to extend the code I have to do that.

Re a), I think Tokyo team can fix this by changing the open/save dialog and Files app to filter out symlinks. 

Re b) I think this should be fixable by changing the file:// handler. Not sure who would be the right owner.


BTW, I don't have access to  issue 677817 . Could you add me to the bug?

vapier: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

I've created symlinks in Downloads and a flash drive formatted in ext4 as follows:

  % cd /home/chronos/user/Downloads
  % ln -s /home/chronos/Local\ State local-state.txt
  % ln -s /home/chronos chronos-dir

  % cd /media/removable/EXT4
  % ln -s /home/chronos/Local\ State local-state.txt
  % ln -s /home/chronos chronos-dir

> 2. Use browser UI to access the files. Known-vulnerable functionality:
>   a. "Save as..." to write through the symlink
>   b. file:// URL to read the target file via the symlink.

Here's what I found:

a) These symlinks did not appear in the Save/Open dialog, and Files app. symlinks are filtered out somewhere already so not exposed to UI.

b) These symlinks did appear in file:///home/chronos/user/Downloads and file:///media/removable/EXT4, and I was able to read /home/chronos/Local\ State and files under files under /home/chronos via the symlinks.

That said, a) isn't a problem but b) needs fixing. Let me have a look

[1]
% cd <downloads-or-flash-drive>
% ln -s /home/chronos/Local\ State local-state.txt
% ln -s /home/chronos chronos-dir

Re "symlinks are filtered out somewhere already" in #7, I think this is the place:

https://cs.chromium.org/chromium/src/storage/browser/fileapi/local_file_util.cc?q=::IsLink&sq=package:chromium&l=57&dr=C

base::FilePath LocalFileEnumerator::Next() {
  base::FilePath next = file_enum_.Next();
  // Don't return symlinks.
  while (!next.empty() && base::IsLink(next))
    next = file_enum_.Next();

+jorgelo

FWIW, here's a quick patch to suppress symlinks in the directory listing, and remove access to files pointed by symlinks from file:// URLs:

https://codereview.chromium.org/2640073003/

In #4, mnissler mentioned that he has a kernel patch to fix this. If that's the way to go, maybe we don't have to make changes in Chrome.

mnissler, and jorgelo, what do you think?

we should do both.  even if the kernel starts enforcing some things, the higher levels should include reasonable checks too.  good security is done in layers :).

Agreed. Also, Satoru, I cc'd you on the other bug that you requested.

vapier and jorgelo: thank you for the feedback. I'll work on the file:// issue, and le me reuse this bug for the file:// issue.

As mentioned in #7, symlinks are not exposed in Save/Open dialog and Files app. 

+eroman per request

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

commit ab97148e15f1fecbdef8f50bed35987d8c6423ab
Author: satorux <satorux@chromium.org>
Date: Thu Jan 26 00:54:43 2017

Remove --allow-file-access and accompanying logic

This flag was added long time ago in crrev.com/65866 for giving
tests unconditional access to files via file://. However, this flag
is no longer necessary because switches::kTestType (--test-type) is
checked in ChromeNetworkDelegate::OnCanAccessFile().

BUG= 677933 
TEST=all tests pass as before

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

[modify] https://crrev.com/ab97148e15f1fecbdef8f50bed35987d8c6423ab/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/ab97148e15f1fecbdef8f50bed35987d8c6423ab/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/ab97148e15f1fecbdef8f50bed35987d8c6423ab/chrome/browser/net/chrome_network_delegate.h
[modify] https://crrev.com/ab97148e15f1fecbdef8f50bed35987d8c6423ab/chrome/common/chrome_switches.cc
[modify] https://crrev.com/ab97148e15f1fecbdef8f50bed35987d8c6423ab/chrome/common/chrome_switches.h
[modify] https://crrev.com/ab97148e15f1fecbdef8f50bed35987d8c6423ab/chrome/test/base/chrome_test_launcher.cc

eroman:

To disallow symlinks, I was originally thinking that we could check if the given path is a symlink in ChromeNetworkDelegate::OnCanAccessFile() [1], but I realized that base::IsLink() is an file operation that's disallowed on IO thread. ChromeNetworkDelegate runs on IO thread, right?

One way to solve this problem is to make OnCanAccessFile() async, but fixing all the call sites can be tedious. Maybe we should do the symlink check elsewhere?

[1] https://codereview.chromium.org/2640073003/

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

commit 7c5360584c37d4d21a46f1c410e1d0d036494bf8
Author: satorux <satorux@chromium.org>
Date: Fri Jan 27 07:24:29 2017

Factor out core logic from OnCanAccessFile() and add tests for it

OnCanAccessFile() had platform-specific logic to decide which paths
are allowed to access via file: schema, that had not tests.
This patch factors out the logic and adds tests for it.

Before adding more code to the logic, I wanted added tests for it.

BUG= 677933 
TEST=the new tests pass

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

[modify] https://crrev.com/7c5360584c37d4d21a46f1c410e1d0d036494bf8/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/7c5360584c37d4d21a46f1c410e1d0d036494bf8/chrome/browser/net/chrome_network_delegate.h
[modify] https://crrev.com/7c5360584c37d4d21a46f1c410e1d0d036494bf8/chrome/browser/net/chrome_network_delegate_unittest.cc

Are we concerned about time of check to time of use races? Any solution that involves separately testing if the path is a link and reading it will have some window for abuse.

Assuming that is acceptable, possible designs to fit it in the current code:

  * Make OnCanAccessFile() async. This has disadvantage of an extra thread hope to file thread.

  * Move the check into URLRequestFileJob. The Job already calls base::GetFileInfo() so knows if it is a symlink. Although less general, you could add a network delegate method that prohibits symlinks, and keep the implementation as part of the generic Job (which already runs on file thread).

  * Instead of making the NetworkDelegate::OnCanAccessFile() async, have it return a callback or interface pointer. Although NetworkDelegate is non-threadsafe (and should be called just from network thread), this callback could be run on the file thread, and passed in the base::FileInfo.

eroman: thank you for the suggestions!

Races are a great point. One idea to avoid the race would be to somehow pass O_NOFOLLOW to net::FileStream to ensure that symlinks are not followed on Chrome OS. Maybe add this flag to base::File::Flags.

https://cs.chromium.org/chromium/src/base/files/file.h?l=65

Played with the idea of O_NOFOLLOW: https://codereview.chromium.org/2670793002

With this patch, a symlink to a file is blocked as intended. 

However, as man 2 open said "symbolic links in earlier components of the pathname will still be followed" with O_NOFOLLOW. That means, if the SD card contains a symlink to "/home/chronos" directory, and the user navigates to file://media/removable/<sdcard>/<symlink>/Local%20State, the contents of /home/chronos/Local\ State will be exposed.

To prevent this, I think we should check if a path contains symbolic links in its parents, that cannot be done atomically with open() thus there will be a race. I'll continue looking tomorrow.

Thanks for exploring O_NOFOLLOW! That is disappointing that it doesn't consider symlinks throughout the entire path.

I wonder if this would work:

  1. Open a file descriptor |fd| to the path (which may be a symlink)
  2. Get the absolute path to |fd| in a non-racy way. I *think* you can readlink() on /proc/self/fd/XXX to get the resolved path
  3. Verify that the absolute path is not in a restricted part of file system
  4. Do subsequent read operations using this verified file descriptor

My assumption here is that /proc/self/fd is a symlink to the fully resolved path, and not just a symlink to the original path passed in. This may be incorrect.

If that doesn't work, I think the most robust approach is to reflect these restricted directories via file permissions, owners, or even separate partitions. If that were the case, you could get the |fd|, then fstat() to check if it is in a restricted part of the system (either checking permissions/owner, or maybe even st_dev to see if it is in a restricted partition).

Beyond that, comment #4 seems the most promising/least hacky. Maybe something racy in the interrim like what we have now is acceptable if it will be addressed robustly later.

eroman: thank you for the idea. As far as I tested locally, /proc/self/fd/X points to the resolved file path, but not sure if it's guaranteed to be so. 'man proc' wasn't clear about it.

Anyway, I like the idea of using the resolved path for checking, because some symlinks are harmless and useful (ex. /home/chornos/log/chrome points to the latest log file in the same directory) so these don't need to be blocked.

If we are going to have #4 in the future, I guess something slightly racy would be acceptable. A simple approach would be to call base:: NormalizeFilePath() in addition to base::GetFileInfo() in URLRequestFileJob::FetchMetaInfo(). Then check the resolved path with OnCanAccessFile() before opening it. 

correction: /home/chornos/log/chrome -> /home/chornos/user/log/chrome

i think using a normalize func on /proc/self/fd/X makes sense (whether that be realpath() or a base:: equiv)

theoretically, i'm not sure we can fully protect ourselves here w/out mnissler's kernel patch.  we have no idea what the device is on the otherside that's plugged in.  it could be a "dumb" thumb drive, or it could be a Linux device (e.g. Android) running a malicious mass storage/file system stack.  it'd be possible to dynamically return different values for a path based on how many times it's been read, so if we did a stat() or readlink() or realpath() on it, it'd first claim to be a real file, and then later claim to be a symlink when we actually try to open() it.  the /proc/ symlink check would catch this if we're only trying to do a read, but if we're trying to open a file for writing, things get tricky.  you can't re-open a fd or change its write mode on the fly afaik.

short of mnissler's patch, i think the only way to protect ourselves is to walk each path component ourselves and use openat for each piece.  so if we wanted to write "subdir/file" we'd do something like:
  dfd1 = open("/media/removable/EXT4", O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW);
  dfd2 = openat(dfd1, "subdir", O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW);
  fd = openat(dfd2, "file", O_RDWR|O_CLOEXEC|O_NOFOLLOW);

the upside is that we wouldn't have to worry about canonicalizing any path (/proc/self/fd/X or otherwise).  the downside is that it's kind of a pain to do all of this.  but maybe a for loop in C++ by walking path components wouldn't be so bad.

Here's another WIP patch based on #24: https://codereview.chromium.org/2674823003/

Because the absolute path is used for checking, a path that includes a symlink in its parent components is blocked correctly. In addition to file access, directory listing is also filtered in the same way.

eroman@, does this patch look good overall? If so, I'm going to polish and add tests.

@satorux: Overall layering looks good! Send me the review when ready.
Per earlier discussion the approach is still racy, but better than nothing.

Thanks! eroman: I'll revise the patch and get back to you.

Sorry for the long absence. I've finally created a new patch:

  https://codereview.chromium.org/2786583002/

I think this is looking good except it broke Clank. :P 

I haven't taken a close look but here's probably what went wrong: In ChromeNetworkDelegate::IsAccessAllowed(), the following paths are whitelisted for Android:


  static const char* const kLocalAccessWhiteList[] = {
      "/sdcard",
      "/mnt/sdcard",
  };

On Android /sdcard and /mnt/sdcard are usually symbolic links to other paths. hirono@ checked his phone and found these point to /storage/self/primary (this can be different). With my patch, paths of requested files are resolved to absolute paths, which don't match /sdcard or /mnt/sdcard anymore.

One way to solve this problem would be to pass both original and absolute paths to NetworkDelegate::OnCanAccessFile(), so that Chrome OS uses absolute ones while Android uses original ones.

Suggestion in c#30 makes sense to me.

> One way to solve this problem would be to pass both original and absolute paths to NetworkDelegate::OnCanAccessFile(), so that Chrome OS uses absolute ones while Android uses original ones.

This didn't work. Will take a closer look.

The following tests in chrome_public_test_apk broke with my change.

org.chromium.chrome.browser.offlinepages.OfflinePageRequestTest#testLoadOfflinePageOnDisconnectedNetwork
org.chromium.chrome.browser.offlinepages.OfflinePageUtilsTest#testShowOfflineSnackbarIfNecessary
org.chromium.chrome.browser.offlinepages.OfflinePageRequestTest#testLoadOfflinePageWithFragmentOnDisconnectedNetwork

The first test was added in https://codereview.chromium.org/2245733004. The change added ability to load cached pages when offline. This ability was implemented by extending URLRequestFileJob:

  class OfflinePageRequestJob : public net::URLRequestFileJob {

With my change, this code broke, because my change added a whitelist check in URLRequestFileJob, that rejects access to cached files.

There is another sub class of net::URLRequestFileJob:

  class URLRequestExtensionJob : public net::URLRequestFileJob {

I think this code also breaks with my change. The tests in bots didn't catch this likely because OnCanAccessFile() returns true if chrome for chromeos runs on Linux desktop.

That said, adding a whitelist check in net::URLRequestFileJob may be a bad idea. :P
I'll have to think about a different approach.

Here's a new idea:

- Add URLRequestFileJob::OnFetchMetadata()
- Create a sub class of URLRequestFileJob for Chrome OS
- In the sub class's OnFetchMetadata(), reject files based on the absolute path
- Use the sub class to check file: URLs with URLRequestInterceptor mechanism

Created a new patch based on the idea at #35 (not polished):

https://codereview.chromium.org/2790073002

eroman@, what do you think about this approach? The idea at #27 didn't work because of #30, #33 and #34. :P

Another approach would be to make an absolute path in ChromeNetworkDelegate::OnCanFile() with ScopedAllowIO on Chrome OS. Doing file IO on UI thread isn't desirable, but for this case it may be OK as file:// URLs aren't performance critical, and the change would be very small.

Moving the discussion back to the bug as this bug as we have more context here. Here's feedback from eroman on https://codereview.chromium.org/2790073002

You will want mmenke to review this change. In fact he can probably advise you
better on the overall approach.

In general I would say we don't want to use an interceptor for this. If we
substitute at this level, how about changing the location a few lines above:

  job_factory->SetProtocolHandler(url::kFileScheme, ...);

And injecting the ChromeOS specific file:// handler there.

That said, I am not sure I understand why the NetworkDelegate based approach did
not work. My understanding is you ran into problems with OfflinePageRequestJob
on Android? I don't know anything about that class, but that would suggest it
isn't using the file job appropriately, or can be modified in how it calls.

Lastly, I should point out that either of these approaches is only going to
allow blacklisting paths from within profile-initiated requests. file://URLs
that come from other contexts may not flow through that. Notably, I expect that
if you specify a file:// URL for a PAC script it will not go through any of
these checks as it is using a different URLRequestContext.

> That said, I am not sure I understand why the NetworkDelegate based approach did not work. My understanding is you ran into problems with OfflinePageRequestJob on Android? I don't know anything about that class, but that would suggest it isn't using the file job appropriately, or can be modified in how it calls.

OfflinePageRequestJob on Android broke with my patch [1] because OfflinePageRequestJob assumes that URLRequestJob does not have any restrictions on paths (i.e. no whitelist checking). I think URLRequestExtensionJob on Chrome OS also broke for the same reason.

Maybe OfflinePageRequestJob and URLRequestExtensionJob shouldn't be using ChromeNetworkDelegate? eroman@, mmenke@, what do you think?


[1] In particular changes to url_request_file_job.cc: https://codereview.chromium.org/2786583002/diff/280001/net/url_request/url_request_file_job.cc

URLRequestFileDirJob appears to have the same problem, so the fix should cover that as well.

I think an interceptor is the wrong approach here as well (Particularly as we're thinking about how to get it out of process, the per-interceptor overhead becomes a lot higher).

Things are a bit funky here because FileProtocolHandler does the initial security check, but we want to an additional check after we've created the job.  So if we don't want the interceptor, we either need to split the checks in different places (One check in the ProtocolHandler, one in the URLRequestFileJob, or its subclass, the the other subclasses in question can disable, or add a callback to check the final path, passed in on construction), or move both checks to the subclasses, and let the subclasses disable them if it so chooses.

My suggestion is to move both checks into URLRequestFileDirJob, and have a virtual method "bool CanAccessPath(const FilePath &path, bool has_sym_links_resolved)", which subclasses can override, and by default calls into the network delegate.

The reason for preferring this approach is that both checks are together, and it forces subclasses to actually think about what to do (I believe all would currently just return true, but forcing people to say that they aren't providing any protection at that layer has value, in my opinion).

It does mean we need to republic code in URLRequestFileDirJob / URLRequestFileJob (Or make them both inherit from the same URLRequestJob subclass).

Thoughts?

"republic code" == replicate code (Or maybe democratic republic code)

mmenke@, thank you for the feedback!


> URLRequestFileDirJob appears to have the same problem, so the fix should cover that as well.

The earlier attempt (one that does not use an interceptor) [1] covers URLRequestFileDirJob as well.

[1] https://codereview.chromium.org/2786583002/ 

> Things are a bit funky here because FileProtocolHandler does the initial security check, but we want to an additional check after we've created the job. <snip> or move both checks to the subclasses

The above mentioned patch removes the check from FileProtocolHandler, but instead added checks in 
URLRequestFileJob and URLRequestFileDirJob

>  let the subclasses disable them if it so chooses.

This is a great idea. I think we can change OfflinePageRequestJob and URLRequestExtensionJob to disable the checks. Let me take another look.

Update: https://codereview.chromium.org/2786583002 PS7 is created based on the comment #42. Hope try bots will pass.

satorux@, friendly ping from security FixIt team. Are you planning to continue work on https://codereview.chromium.org/2786583002 ? Can I help you anyhow?

Sorry I was swamped with other things lately, and will be ooo this week. Let me try to resume next week.

Let's put this up as NextAction on Tuesday next week, and update the milestone.

Uploaded https://codereview.chromium.org/2786583002/ and requested a review.

BTW, while I've been slow on this, mnissler@ made lots of progress on the kernel approach:

https://groups.google.com/a/chromium.org/forum/?hl=en#!topic/chromium-os-dev/y2289jINZ9w

Re comment #48: I've indeed made progress on blocking symlink traversal in the kernel. However, the plan is to do this selectively, and I'm aiming only for stateful and encrypted stateful in a first push. Specifically, home directories and external storage media as relevant for this bug are not covered by the system-level symlink traversal blocking effort for now.

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

commit 4e5aa748d7b05a4057b1960585c3a5b5bc52561a
Author: satorux <satorux@chromium.org>
Date: Wed May 17 06:30:30 2017

chromeos: Remove kTestType conditional from ChromeNetworkDelegate

The conditional appears to be no longer necessary in Chrome's tests.

Per "repo grep -- --test-type" in chromeos tree, the flag appears not
to be passed to chrome in auto tests.

BUG= 677933 
TEST=all linux_chromium_chromeos bots passed

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

[modify] https://crrev.com/4e5aa748d7b05a4057b1960585c3a5b5bc52561a/chrome/browser/net/chrome_network_delegate.cc

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

commit 561da39a7a22b9d1ff96fcc742bd32bc2c3543b6
Author: satorux <satorux@chromium.org>
Date: Tue May 23 06:29:44 2017

chromeos: Remove blanket file access logic from chromeos chrome on Linux

crrev.com/472352 removed kTestType conditional from ChromeNetworkDelegate
for Chrome OS, but this change adds it back, and instead remove blanket file
access logic based on IsRunningOnChromeOS().

This blanket file access logic was problematic when it comes to write an end-to-end
browser test that ensures access control of file: scheme is working correctly
(i.e. with this logic, browser_tests for Chrome OS that runs on Linux desktop can
access to any files).

If you need to load your files with Chrome for Chrome OS on Linux
desktop [1], please place these files under /tmp, that's white listed
in ChromeNetworkDelegate::IsAccessAllowed().

[1] This build config is for development only, not for production.

BUG= 677933 
TEST=access to file:///tmp is allowed on Linux desktop, but file:/// is not

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

[modify] https://crrev.com/561da39a7a22b9d1ff96fcc742bd32bc2c3543b6/chrome/browser/net/chrome_network_delegate.cc

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

commit ddac044c41e8b7a223c403e4574f5d44dff5e50e
Author: satorux <satorux@chromium.org>
Date: Mon May 29 06:06:18 2017

chromeos: Check both original and absolute paths for file: scheme

Previously, paths not in the whitelist were accessible through
symbolic links via file: scheme. This patch solves this
problem by checking both original and absolute paths (i.e.
symbolic link is resolved with the latter).

It is a known issue that this approach has a race between
time to check the resolved path and time to read the path.

BUG= 677933 
TEST=confirm that paths not in the whitelist cannot be accessed via symbolic links on Chrome OS
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

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

[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/android_webview/browser/net/aw_network_delegate.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/android_webview/browser/net/aw_network_delegate.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/chrome/browser/android/offline_pages/offline_page_request_job.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/chrome/browser/android/offline_pages/offline_page_request_job.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/chrome/browser/net/chrome_network_delegate.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/chrome/browser/net/chrome_network_delegate_unittest.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/chromecast/browser/cast_network_delegate.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/chromecast/browser/cast_network_delegate.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/components/cronet/android/cronet_url_request_context_adapter.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/content/shell/browser/shell_network_delegate.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/content/shell/browser/shell_network_delegate.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/extensions/browser/extension_protocols.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/headless/lib/browser/headless_network_delegate.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/headless/lib/browser/headless_network_delegate.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/ios/chrome/browser/net/ios_chrome_network_delegate.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/ios/chrome/browser/net/ios_chrome_network_delegate.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/ios/web/shell/shell_network_delegate.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/ios/web/shell/shell_network_delegate.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/ios/web_view/internal/web_view_network_delegate.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/ios/web_view/internal/web_view_network_delegate.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/base/directory_lister.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/base/directory_lister.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/base/layered_network_delegate.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/base/layered_network_delegate.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/base/layered_network_delegate_unittest.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/base/network_delegate.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/base/network_delegate.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/base/network_delegate_impl.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/base/network_delegate_impl.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/proxy/network_delegate_error_observer_unittest.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/proxy/proxy_script_fetcher_impl_unittest.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/url_request/file_protocol_handler.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/url_request/url_request_file_dir_job.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/url_request/url_request_file_dir_job.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/url_request/url_request_file_job.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/url_request/url_request_file_job.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/url_request/url_request_test_util.cc
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/url_request/url_request_test_util.h
[modify] https://crrev.com/ddac044c41e8b7a223c403e4574f5d44dff5e50e/net/url_request/url_request_unittest.cc

I'm working on a browser test for this now.

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

commit ca8b564942b90a8ce527933dca71645880e214ab
Author: satorux <satorux@chromium.org>
Date: Thu Jun 08 04:26:35 2017

chromeos: Replace "/tmp" with a call to PathService::Get()

This is in preparation for adding a browser test that is to
ensure access control for file:scheme is working. In the
browser test, I plan to use PathService::Get(DIR_TEMP) to
store a symbolic link in a location that's accessible via file:
scheme.

Confirmed that PathService::Get(DIR_TEMP) returns /tmp in
production so this patch does not change the production
behavior.

Along the way, simplify the code a bit by introducing a vector.

TEST=file:///tmp is still accessible on a Chromebook
BUG= 677933 

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

[modify] https://crrev.com/ca8b564942b90a8ce527933dca71645880e214ab/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/ca8b564942b90a8ce527933dca71645880e214ab/chrome/browser/net/chrome_network_delegate_unittest.cc

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

commit d18e61ab5e01b85ff9160c4ab143cba6cd64d7be
Author: satorux <satorux@chromium.org>
Date: Thu Jun 08 06:38:46 2017

Add a browser test for access control for file: scheme

Access to files via file: scheme is controled in
ChromeNetworkDelegate::OnCanAccessFile(). The browser
test is to ensure that this function is actually used
to control the access.

BUG= 677933 
TEST=the test passes on bots
NOPRESUBMIT=true

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

[modify] https://crrev.com/d18e61ab5e01b85ff9160c4ab143cba6cd64d7be/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/d18e61ab5e01b85ff9160c4ab143cba6cd64d7be/chrome/browser/net/chrome_network_delegate.h
[add] https://crrev.com/d18e61ab5e01b85ff9160c4ab143cba6cd64d7be/chrome/browser/net/chrome_network_delegate_browsertest.cc
[modify] https://crrev.com/d18e61ab5e01b85ff9160c4ab143cba6cd64d7be/chrome/browser/net/errorpage_browsertest.cc
[modify] https://crrev.com/d18e61ab5e01b85ff9160c4ab143cba6cd64d7be/chrome/test/BUILD.gn
[modify] https://crrev.com/d18e61ab5e01b85ff9160c4ab143cba6cd64d7be/chrome/test/base/in_process_browser_test.cc
[modify] https://crrev.com/d18e61ab5e01b85ff9160c4ab143cba6cd64d7be/content/public/test/test_navigation_observer.cc
[modify] https://crrev.com/d18e61ab5e01b85ff9160c4ab143cba6cd64d7be/content/public/test/test_navigation_observer.h

Fixed in M61 with a browser test with a massive amount of help from mmenke@. :)

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This is weird, I don't think anything needs merged, the last CL landed in June, and should already be in 62.

(the VRP panel considered this bug when rewarding for  issue 677934 )