Bug 154345 – CSP: Implement frame-ancestors directive

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Daniel Bates]

We should implement the Content Security Policy frame-ancestors directive to allow a web developer to whitelist the resources that are allowed to be embedded in a web page. In an HTML document such embedding may be via the use of a frame, iframe, object, embed, or applet element. See <https://w3c.github.io/webappsec-csp/2/#directive-frame-ancestors> for more details.

[Radar WebKit Bug Importer]

<rdar://problem/24702161>

[Daniel Bates]

Created attachment 273630 [details]
Patch and Layout Tests

[Daniel Bates]

Created attachment 273636 [details]
Patch and Layout Tests

[Brent Fulgham]

Comment on attachment 273636 [details]
Patch and Layout Tests

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

r=me.

> Source/WebCore/loader/FrameLoader.cpp:691
> +        m_frame.document()->contentSecurityPolicy()->didReceiveHeaders(m_documentLoader->response(), ContentSecurityPolicy::ReportParsingErrors::No);

So we are using an implicit cast here? I'm not sure that is an improvement.

> Source/WebCore/page/csp/ContentSecurityPolicy.h:165
> +    // We can never have both a script execution context and a frame.

Seems like we should ASSERT on this invariant, perhaps in places where we set one of these values. That is not needed for the current patch, but it's something to consider in the future.

[Daniel Bates]

(In reply to comment #4)
> > Source/WebCore/loader/FrameLoader.cpp:691
> > +        m_frame.document()->contentSecurityPolicy()->didReceiveHeaders(m_documentLoader->response(), ContentSecurityPolicy::ReportParsingErrors::No);
> 
> So we are using an implicit cast here? I'm not sure that is an improvement.
> 

Will revert. I will also mark the constructor of ContentSecurityPolicyResponseHeaders explicit and update all other callers.

I take it you feel that it is makes the code more understandable to see the explicit construction of a ContentSecurityPolicyResponseHeaders object from a ResourceResponse object.

> > Source/WebCore/page/csp/ContentSecurityPolicy.h:165
> > +    // We can never have both a script execution context and a frame.
> 
> Seems like we should ASSERT on this invariant, perhaps in places where we
> set one of these values.

No need to use a run-time assert. This invariant is enforced by the public interface of ContentSecurityPolicy. It is not possible to create a ContentSecurityPolicy object that has both a script execution context and a frame because you can only instantiate one or the other via two different constructors and ContentSecurityPolicy does not expose a setter for either ContentSecurityPolicy::m_scriptExecutionContext or ContentSecurityPolicy::m_frame.

[Daniel Bates]

Committed r197972: <http://trac.webkit.org/changeset/197972>