Thanks for filing this as a separate issue. I'm tentatively assigning severity-medium to this, but feel free to change as you see appropriate.

lazyboy@ added crash keys in r396275 to find out which apps are responsible for these kills.

lazyboy@: Have you looked at any of the data yet?  I'm curious how many are affected.

With help from creis@, I have seen 17 crashes so far
query: https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27browser%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27%5BRenderer%20kill%5D%20content%3A%3AAppCacheInterceptor%3A%3ACompleteCrossSiteTransfer%27%20AND%20product.version%3E%3D%2753.0.2750.0%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D&stbtiq=&reportid=&index=0

16 out of 17 are from Weather Bug, extension id: njkkjobcechefaoknodniidfjapgfoco
1 is from Magisto - Magical Video Editor, extension id: ghmngbmfdgknokcefmkbjlcjabdklnlk (370k users)

Magisto app is also loading a web iframe in a sandbox page AFAICT from the source.

Some more crashes can be found here, the crashes are split between the two extensions I've mentioned @3 above:
https://dashboards.corp.google.com/#/google::_9618a801_f12a_46c3_821a_ca664123f024

However, one of the reports
https://crash.corp.google.com/browse?q=reportid=%27ff88f6aa00000000%27#4
recorded www.google.com as the host, which I cannot explain yet.

Another one from your dashboard: capmpnebnckdpjbjdcgnokjafpdbmadn (Pong 2)

I wouldn't worry about the one that listed google.com unless we see more web sites.  That may be some other bug.

Note: We no longer try to load these web iframes in a different process from V2 apps in --isolate-extensions, so the renderer kills in issue 612711 are gone (apart from any users of --site-per-process).

lazyboy@: What's the next step for the deprecation?

OK, so far we have 3 extensions and I haven't been able to finish the rappor CL.

I can start working on deprecating sandbox pages loading web content and send FYI notes to the owners of those extensions.

I'd like to know more about loading scripts though. One useful use case asargent@ mentioned was to able to "eval" stuff on isolated world safely. Can you explain why they are as dangerous as loading web content? and how they are different than foo.com loading jquery library from another host? Maybe a separate bug to track that? Thanks.

As far as I recall, the security model for Apps v2 is that all of its code is contained in the package. It gives us auditability and it is also performant, as one can launch the app without the need for internet connectivity.

I think it is this requirement that implies no eval of downloaded code. Apps are by nature different than the open web, hence the restrictions we've put on them.

Today's crash keys numbers are as follows [1]:

www.google.com
5
ghmngbmfdgknokcefmkbjlcjabdklnlk (cannot find in webstore or cs.chromium.org)
3
www.paypal.com
3
njkkjobcechefaoknodniidfjapgfoco (Weather Bug)
140
capmpnebnckdpjbjdcgnokjafpdbmadn (Pong 2)
54
cnciopoikihiagdjbjpnocolokfelagl (Videostream for Google Chromecast™)
28


Given we are aware of (only) 3 extensions that is going to break when we disallow sandbox, should we just go ahead with it now? Of course with sending FYI emails to the developers of the extensions.

[1] dashboard query: SELECT v, t, e FROM(SELECT product.Version AS v, ProductData.value AS e, SUM(1) AS t FROM crash.prod.latest WHERE custom_data.ChromeCrashProto.ptype='browser' AND custom_data.ChromeCrashProto.magic_signature_1.name='[Renderer kill] content::AppCacheInterceptor::CompleteCrossSiteTransfer' GROUP BY v, e HAVING SUM(ProductData.key='aci_wrong_sp_extension_id')>0)

Those apps have a decent number of users.  Can we reach out to them a little bit before we fix this, and make sure they can update (and if not, do we need to add another option for them)?  creis@, is that a security risk (or perhaps better phrased as "an acceptable security risk")?  Obviously we don't need to go into huge detail about the reason why, but there's always the chance they could figure it out.

Has any progress been made on working with the affected apps?

Comment 11: I think it's fine to reach out to them to say that the behavior is being deprecated in favor of <webview>.  I'd encourage it to make this smooth for them, and to get us into a position where we can lock this down ASAP.

OK, can you guys check if sth like this is fine?

"""
Hi,
I work on chrome browser's extension system and I'm letting you know that your XXX app in chrome webstore uses sandbox pages (either or both of sb.html and sb2.html*) to load 3rd party web (http/https) content.
We are currently working on deprecating sandbox pages to load 3rd party web content in favor of a better and secure existing alternative to load 3rd party web content in chrome apps: <link>webview</link>. Note that this doesn't affect loading extension local files/content inside sandbox pages.

It'd be better if you could switch to using webview instead and help us move forward with the deprecation.

Thank you!
"""

* sb.html/sb2.html is a particular example I pulled from ghmngbmfdgknokcefmkbjlcjabdklnlk
ghmngbmfdgknokcefmkbjlcjabdklnlk wasn't accessible before, but I can access it now, though it is unpublished state.

It's probably worth having someone from DevRel proofread it if possible.  The general message seems fine, but they can probably help with the wordsmithing.

Minor things like capitalizing Chrome, how to phrase the Web Store, how to refer to the sandboxed pages feature, and how to phrase the request (e.g., "We're hoping that you will be able to migrate from sandboxed pages to <webview> as we move forward with the deprecation.  We're happy to answer questions on how to move to <webview>.").

We should also be clear that the sandboxed pages feature isn't being deprecated from extensions, only from apps.

Thanks for making progress on it!

Adding Hotlist-DevRel, see comment #13, #14 and #15 for context.

Have we started the reaching out? I would like to see us close this bug as soon as possible.

Added DevRel hotlist to get attention for proof reading... I pinged Meggin about this bug, didn't get any response.
Maybe I should just go ahead with what I have + what Charlie suggested (see #14 and #15)

Hi, all.

How's this?

Hi,

My name is XXX and I work on the Chrome browser's Apps and Extension system. 

This email is to reach out to you to let you know that <link>sandboxing<link>
in Chrome Apps will soon be deprecated.

We request that your XXX app in the Chrome Web Store be updated to use
<link>webview<link> instead of sandboxing to link to 3rd party web content.

Using a webview to load 3rd party web content is a more secure alternative. Please help us move forward with the sandboxing deprecation.

If you need any assistance,
let us know and we will do our best to help: contact-info.

Thanks!

comment 21: Thanks!  That helps a lot.  My main request would be to say "sandboxed pages" instead of "sandboxing," to avoid any ambiguity with Chrome's sandbox.  (We don't want to sound like we're deprecating that!)  :)

Also, s/link to 3rd party/load 3rd party/ in sentence 3.  Otherwise sounds perfect.  Thanks!

Ping.  Has this gone out yet?  Thanks in advance for any updates!

Yes I've sent the emails out ~2 weeks ago.

Did we get any response? Can we make these changes in M56?

No response. We should just try landing the changes, WDYT?

sgtm.

sgtm too

Any progress on this? The goal was to start landing changes about a month ago. Has anything happened here?

No sorry, I haven't. Looked into details a bit just now.

One way we could accomplish this is to harden the CSP of sandbox manifest [1]
to contain child-src/script-src/object-src set to 'self'

This would require rewriting the CSP in
bool ContentSecurityPolicyIsSandboxed() [2] which honestly doesn't seem too clean to me as we'd have to parse CSP string.

Then we could just harden the relaxed condition we put in
ChromeContentBrowserClientExtensionsPart::DoesSiteRequireDedicatedProcess() [3].

Any other ideas welcome.

[1] https://developer.chrome.com/apps/manifest/sandbox
[2] https://cs.chromium.org/chromium/src/extensions/common/csp_validator.cc?rcl=1479846071&l=340
[3] https://cs.chromium.org/chromium/src/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc?sq=package:chromium&dr=CSs&rcl=1479846071&l=295

I don't know what the best approach is, but I do know that we hold ourselves to higher standards on fixing security bugs. This being a P1 should've been fixed long time ago. Can we please prioritize this higher?

@lazyboy, thanks for the CL at https://codereview.chromium.org/2563843002/ - looks like a good approach.

Did we ever send out an email blast to chromium-apps@?  I know you reached out to developers directly, but we should make a wide announcement if we can.

nasko@/creis@, any concern about that?  Theoretically it could make it more widely known that there's a security issue here, but since we have a CL up, it won't be secret for long.

Yes, I'm fine with a deprecation announcement similar to what we sent in comments 21-22, talking deprecating it without spelling out the exact security concerns.

Thanks for getting the CL going-- I agree that we should get this resolved ASAP.

comment #34, there were no deprecation announcements sent out to mailing list, I can send them out. Probably *after* the CL lands?

right after landing sgtm.

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

commit c0304f6696623d76672a4c8fbefa16378c3137b8
Author: lazyboy <lazyboy@chromium.org>
Date: Tue Jan 03 21:17:30 2017

Restrict app sandbox's CSP to disallow loading web content in them.

The CL restricts loading subframes and scripts inside chrome app
sandbox pages (https://developer.chrome.com/apps/manifest/sandbox).
The restriction would only allow loading these resources from
'self', i.e. the app's resources.

1) scripts
Two CSP directives can affect subframe restriction:
script-src or
default-src fallback when script-src is not present.

2) subframes
Three possible CSP directives can affect script restriction:
frame-src or (deprecated)
child-src or
default-src fallback when none of frame-src/child-src is present.

This CL adds a GetEffectiveSandboxedPageCSP function, which
returns the effective CSP chrome will use when a certain
sandbox CSP (sandbox.content_security_policy key) is specified in
app's manifest.
The effective CSP is a restricted CSP that strips out any remote
hosts from #1 and #2. Any directive source value that is not
surrounded by single quotes are considered remote hosts. The
approach is to add 'self' as source value by default if 'none' was
not specified.

TODO: Update sandbox documentation page.

BUG= 615585 
Test=Loading a remote script from foo.com inside a chrome
apps sandbox page should not be possible, even with setting
the following "sandbox.content_security_policy" key in the
manifest:
script-src: foo.com
Same would go with loading <iframe> elements. It should not
be possible to load web content <iframe>s inside apps.

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

[modify] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
[modify] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/chrome/browser/extensions/sandboxed_pages_apitest.cc
[modify] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/chrome/common/extensions/docs/templates/articles/manifest/sandbox.html
[add] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/chrome/test/data/extensions/api_test/sandboxed_pages_csp/local_frame.html
[add] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/chrome/test/data/extensions/api_test/sandboxed_pages_csp/local_frame.js
[add] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/chrome/test/data/extensions/api_test/sandboxed_pages_csp/main.html
[add] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/chrome/test/data/extensions/api_test/sandboxed_pages_csp/main.js
[add] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/chrome/test/data/extensions/api_test/sandboxed_pages_csp/manifest.json
[add] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/chrome/test/data/extensions/api_test/sandboxed_pages_csp/remote_frame.html
[add] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/chrome/test/data/extensions/api_test/sandboxed_pages_csp/remote_frame.js
[add] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/chrome/test/data/extensions/api_test/sandboxed_pages_csp/sandboxed.html
[modify] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/extensions/common/csp_validator.cc
[modify] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/extensions/common/csp_validator.h
[modify] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/extensions/common/csp_validator_unittest.cc
[modify] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/extensions/common/manifest_handlers/csp_info_unittest.cc
[modify] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/extensions/common/manifest_handlers/sandboxed_page_info.cc
[modify] https://crrev.com/c0304f6696623d76672a4c8fbefa16378c3137b8/extensions/test/data/manifest_tests/sandboxed_pages_valid_3.json

Any more changes expected for this issue, or is it OK to mark as fixed?  Also I presume that you want to keep this in M57 and not merge to M56 seeing as the final beta is this week.

Should be marked as fixed. Yes, only for M57.

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

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

commit 066b6017317931d5975f81a8bf42ebe4903d9fa0
Author: Nasko Oskov <nasko@chromium.org>
Date: Thu Feb 08 20:52:06 2018

Remove obsolete PlatformAppsNotIsolated test.

Since  https://crbug.com/615585  is fixed, this test is obsolete and can
be removed.

Bug:  615585 
Change-Id: Icea5ebb0d47215ee35d0d39ee2ccf6eb5aa4843e
Reviewed-on: https://chromium-review.googlesource.com/909304
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535505}
[modify] https://crrev.com/066b6017317931d5975f81a8bf42ebe4903d9fa0/chrome/browser/site_details_browsertest.cc