Features/Platform/Iframe Sandbox: Difference between revisions

no edit summary
No edit summary
No edit summary
 
(2 intermediate revisions by 2 users not shown)
Line 1: Line 1:
{{FeatureStatus
{{FeatureStatus
|Feature name=Iframe Sandbox
|Feature name=Iframe Sandbox
|Feature stage=Development
|Feature stage=Landed
|Feature status=In progress
|Feature status=Complete
|Feature version=Firefox 17
|Feature health=OK
|Feature health=OK
|Feature status note=initial implementation done, initial test suite done, working through some hopefully final design decisions
|Feature status note=landed in FF17 - some followup work remains
}}
}}
{{FeatureTeam
{{FeatureTeam
Line 29: Line 30:
* We will create the flags as described in the HTML5 spec's description of the IFRAME sandbox attribute on both the docshell and the document when it is loaded  
* We will create the flags as described in the HTML5 spec's description of the IFRAME sandbox attribute on both the docshell and the document when it is loaded  


(the flag constants will move to their own header and out of nsIDocShell in the near future)
content/base/src/nsSandboxFlags.h contains :  
nsIDocShell.idl will contain :  
<pre>
<pre>
+  /**
+  /**
Line 85: Line 85:
* for CSP sandbox, the flags will only be stored on the document itself - when content is navigated to, the CSP sandbox flags won't be persisted (unless the new content also specifies a CSP sandbox directive)
* for CSP sandbox, the flags will only be stored on the document itself - when content is navigated to, the CSP sandbox flags won't be persisted (unless the new content also specifies a CSP sandbox directive)
* the HTML5 spec provides examples of how to apply flags with nested IFRAMEs, abarth has proposed that if both CSP and IFRAME sandbox can apply to content, the algorithm used in these example should be used to merge the policies which sounds reasonable
* the HTML5 spec provides examples of how to apply flags with nested IFRAMEs, abarth has proposed that if both CSP and IFRAME sandbox can apply to content, the algorithm used in these example should be used to merge the policies which sounds reasonable
* Disabling Javascript via  mDocShell->SetAllowJavascript(false) causes some issues, for example, video controls and even the error for an unsupported codec don't work in this situation. I want to try an alternate implementation that uses the same script choke points as CSP to try to maximize the functionality available in a sandboxed document while still not allowing the document to load a remote script or execute inline script.
* Disabling Javascript uses the same script choke points as CSP to maximize the functionality available in a sandboxed document while still not allowing the document to load a remote script or execute inline script.
* After discussion, for workers, the plan is to allow workers to be loaded inside a sandboxed document with 'allow-scripts' (but not to require 'allow-same-origin') from a data: URL or a blob URL created by the same sandboxed document creating the worker. This requires modifying either worker code or CheckMayLoad() code most likely.  
* After discussion, we allow workers to be loaded inside a sandboxed document with 'allow-scripts' (but without 'allow-same-origin') from a data: URL or a blob URL created by the same sandboxed document creating the worker. This requires modifying either worker code or CheckMayLoad() code most likely.  
* After discussion, although the sandbox attribute is specified as DOMSettableTokenList in the HTML5 spec, this implementation will implement it as a DOMString.
* After discussion, although the sandbox attribute is specified as DOMSettableTokenList in the HTML5 spec, this implementation implements it as a DOMString.
|Feature security review=This feature will definitely need a full security review from the Security Assurance team. I've posted this feature page on dev.security and updated the bug with decisions and implementation plans as implementation has proceeded. Before the security review I will post again to dev.security and encourage review of this feature page and the HTML5 iframe sandbox spec.
|Feature security review=This feature will definitely need a full security review from the Security Assurance team. I've posted this feature page on dev.security and updated the bug with decisions and implementation plans as implementation has proceeded. Before the security review I will post again to dev.security and encourage review of this feature page and the HTML5 iframe sandbox spec.
|Feature qa review=We will need a test suite for this feature. Microsoft has released test cases for sandboxing publically that have been submitted to the W3C for inclusion in the HTML5 test suite. We will definitely want to compare our implementation to other browsers' implementation for consistency etc. and address inconsistencies via suggested modifications to the HTML5 spec and discussion on the whatwg list. Boris Zbarsky has suggested submitting our sandbox test suite to the W3C also.
|Feature qa review=We will need a test suite for this feature. Microsoft has released test cases for sandboxing publically that have been submitted to the W3C for inclusion in the HTML5 test suite. We will definitely want to compare our implementation to other browsers' implementation for consistency etc. and address inconsistencies via suggested modifications to the HTML5 spec and discussion on the whatwg list. Boris Zbarsky has suggested submitting our sandbox test suite to the W3C also.


A mochitest test suite will be written to provide consistent automated testing for this feature.
A mochitest test suite has been written to provide consistent automated testing for this feature.
|Feature implementation notes=See https://bugzilla.mozilla.org/show_bug.cgi?id=341604 for a patch that attempts to implement HTML5 iframe sandbox.  
|Feature implementation notes=See https://bugzilla.mozilla.org/show_bug.cgi?id=341604 for a patch that attempts to implement HTML5 iframe sandbox.  


There's an initial set of tests in that bug as well.
There's an set of tests for various pieces of functionality in that bug as well.


Please see the bug for updates to current status and remaining implementation issues/tasks.
Please see the bug for updates to current patches and iteration based on feedback/review.
|Feature landing criteria=* Needs a test suite
|Feature landing criteria=* Needs a test suite - DONE
* Needs to be compared against other implementations and public test suites for consistency
* Needs to be compared against other implementations and public test suites for consistency - DONE
* Needs a full security review
* Needs a full security review - DONE
}}
}}
{{FeatureInfo
{{FeatureInfo
Line 109: Line 109:
}}
}}
{{FeatureTeamStatus
{{FeatureTeamStatus
|Feature engineering status=In Progress
|Feature engineering status=Complete
|Feature engineering notes=Initial implementation done, has received feedback from 3 people. Experimental prototype was rewritten after initial feedback. Test suite has been written. Working through remaining implementation issues, then will seek out another round of feedback on the tests and the implementation itself.
|Feature engineering notes=Landed in FF17 - there are some followups.
 
}}
}}
Confirmed users
197

edits