Bug 171327

Summary: Add attribute allow-top-navigation-by-user-activation to iframe sandbox
Product: WebKit Reporter: Bin Lu <binlu>
Component: FramesAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: binlu, buildbot, cdumez, dbates, dvoytenko, esprehn+autocc, fred.wang, jond, kangil.han, malteubl, mkwst
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 174351    
Bug Blocks: 175300, 182248    
Attachments:
Description Flags
Patch
buildbot: commit-queue-
Archive of layout-test-results from ews126 for ios-simulator-wk2
none
Patch
none
Adjustment after bug 174351
none
Patch cdumez: review+, cdumez: commit-queue-

Description Bin Lu 2017-04-26 09:12:35 PDT
There is a new attribute proposed to iframe sandbox:
https://html.spec.whatwg.org/multipage/browsers.html#attr-iframe-sandbox-allow-top-navigation-by-user-activation

This is a follow-up work of:
https://bugs.webkit.org/show_bug.cgi?id=158875
https://bugs.webkit.org/show_bug.cgi?id=171321

The new attribute requires a user activation (or gesture) being processed to trigger a top-level navigation. This change would enable more use cases of sandboxing untrusted third-party contents (eg., ads) by allowing top navigation while blocking malicious auto-redirecting, and thus help building a safer internet (eg., a safer ads ecosystem in which all ads could be sandboxed to prevent unexpected malicious behaviors like plugin exploits, auto-redirects, file downloading, modal dialogs, etc). 

Demo link (Available in Chrome 58+):
http://w3c-test.org/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_allow_top_navigation_by_user_activation-manual.html

For more context:
https://github.com/w3ctag/design-reviews/issues/154
https://github.com/WICG/interventions/issues/42
Comment 1 Frédéric Wang (:fredw) 2017-06-21 09:53:41 PDT
There is also an automated test:
http://w3c-test.org/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_allow_top_navigation_by_user_activation_without_user_gesture.html

I'm not sure, but maybe it is going to timeout for the same reason as tests mentioned in bug 173657.
Comment 2 Frédéric Wang (:fredw) 2017-07-11 02:53:48 PDT
Created attachment 315087 [details]
Patch
Comment 3 Build Bot 2017-07-11 04:32:00 PDT
Comment on attachment 315087 [details]
Patch

Attachment 315087 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4099774

New failing tests:
http/tests/security/frameNavigation/sandbox-ALLOWED-top-navigation-with-user-gesture-1.html
http/tests/security/frameNavigation/sandbox-ALLOWED-top-navigation-with-user-gesture-2.html
Comment 4 Build Bot 2017-07-11 04:32:02 PDT
Created attachment 315090 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 5 Frédéric Wang (:fredw) 2017-07-11 06:49:16 PDT
Created attachment 315103 [details]
Patch

New version using UIHelper.
Comment 6 Frédéric Wang (:fredw) 2017-07-11 06:49:45 PDT
Created attachment 315104 [details]
Adjustment after bug 174351
Comment 7 Frédéric Wang (:fredw) 2017-07-12 08:50:58 PDT
Created attachment 315246 [details]
Patch
Comment 8 Chris Dumez 2017-07-17 11:48:21 PDT
Comment on attachment 315246 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315246&action=review

r=me

> Source/WebCore/dom/Document.cpp:3104
> +    // i. A frame can navigate its top ancestor when its 'allow-top-navigation' flag is set (sometimes known as 'frame-busting')

WebKit comments should end with a period.

> LayoutTests/ChangeLog:19
> +        * http/tests/security/frameNavigation/sandbox-ALLOWED-top-navigation-with-user-gesture-2.html: Added.

Please add a test for when such navigation is NOT allowed.
Comment 9 Frédéric Wang (:fredw) 2017-07-21 01:45:19 PDT
Comment on attachment 315246 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315246&action=review

>> Source/WebCore/dom/Document.cpp:3104
>> +    // i. A frame can navigate its top ancestor when its 'allow-top-navigation' flag is set (sometimes known as 'frame-busting')
> 
> WebKit comments should end with a period.

oops, this change is a mistake indeed.

>> LayoutTests/ChangeLog:19
>> +        * http/tests/security/frameNavigation/sandbox-ALLOWED-top-navigation-with-user-gesture-2.html: Added.
> 
> Please add a test for when such navigation is NOT allowed.

OK, I'll do that. Thanks!
Comment 10 Frédéric Wang (:fredw) 2017-07-24 01:33:21 PDT
Committed r219797: <http://trac.webkit.org/changeset/219797>
Comment 11 Bin Lu 2017-11-08 09:11:08 PST
Any idea on when this will be available on which version of Safari?

I tested it on Safari on iOS 11/11.1, as well as Safari 11/11.01 on macOS, and it's not recognized (although 'allow-popups-to-escape-sandbox' is supported).
But Safari technology preview version 40 does support it. So I'm wondering how Safari technology preview version would correspond to Safari version or iOS version.

Thanks!
Comment 12 Frédéric Wang (:fredw) 2018-03-30 12:08:36 PDT
(In reply to Bin Lu from comment #11)
> Any idea on when this will be available on which version of Safari?

Hi Bin Lu. I just tested with the latest releases of iOS and macOS and the allow-top-navigation-by-user-activation works for me.

Tests:
http://w3c-test.org/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_allow_top_navigation_by_user_activation-manual.html
http://w3c-test.org/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_allow_top_navigation_by_user_activation_without_user_gesture.html
https://webkit.org/demos/frames/sandboxing/
Comment 13 Bin Lu 2018-04-02 12:11:57 PDT
Awesome. Thanks Frederic!

I've just verified that "allow-top-navigation-by-user-activation" is now supported on iOS 11.3. For MacOS, I haven't been able to get the Safari update yet, and will test it once I get it.
Comment 14 Bin Lu 2018-04-19 16:33:30 PDT
My MacOS has been finally updated, and I've just verified that "allow-top-navigation-by-user-activation" is now supported on Safari 11.1 (13605.1.33.1.2) on MacOS High Sierra Version 10.13.4.

Thanks Frederic again for the nice work!