Changeset 244589 in webkit


Ignore:
Timestamp:
Apr 24, 2019 8:43:09 AM (5 years ago)
Author:
Chris Dumez
Message:

X-Frame-Options header should be ignored when frame-ancestors CSP directive is present
​https://bugs.webkit.org/show_bug.cgi?id=197226
<rdar://problem/50155649>

Reviewed by Alex Christensen.

Source/WebCore:

X-Frame-Options header should be ignored when frame-ancestors CSP directive is present:

Specification says:
"""
In order to allow backwards-compatible deployment, the frame-ancestors directive _obsoletes_ the
X-Frame-Options header. If a resource is delivered with an policy that includes a directive named
frame-ancestors and whose disposition is "enforce", then the X-Frame-Options header MUST be ignored.
"""

Gecko and Blink follow the specification, WebKit does not. As a result, page [1] is broken with
WebKit-only on Schwab.com. The page height is wrong and you cannot see all the ETFs as a result.

[1] ​https://www.schwab.com/public/schwab/investing/investment_help/investment_research/etf_research/etfs.html?&path=/Prospect/Research/etfs/overview/oneSourceETFs.asp

Test: http/tests/security/contentSecurityPolicy/1.1/frame-ancestors/frame-ancestors-overrides-X-Frames-Options.html

  • loader/DocumentLoader.cpp:

(WebCore::DocumentLoader::responseReceived):

  • page/csp/ContentSecurityPolicy.cpp:

(WebCore::ContentSecurityPolicy::overridesXFrameOptions const):

  • page/csp/ContentSecurityPolicy.h:
  • page/csp/ContentSecurityPolicyDirectiveList.h:

(WebCore::ContentSecurityPolicyDirectiveList::hasFrameAncestorsDirective const):

Source/WebKit:

  • NetworkProcess/NetworkResourceLoader.cpp:

(WebKit::NetworkResourceLoader::shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions):

LayoutTests:

Add layout test coverage.

  • http/tests/security/contentSecurityPolicy/1.1/frame-ancestors/frame-ancestors-overrides-X-Frames-Options-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/1.1/frame-ancestors/frame-ancestors-overrides-X-Frames-Options.html: Added.
  • http/tests/security/contentSecurityPolicy/resources/frame-ancestors-self-x-frame-options-deny.pl: Added.
Location:
trunk
Files:
3 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r244582 r244589  
     12019-04-24  Chris Dumez  <cdumez@apple.com>
     2
     3        X-Frame-Options header should be ignored when frame-ancestors CSP directive is present
     4        https://bugs.webkit.org/show_bug.cgi?id=197226
     5        <rdar://problem/50155649>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Add layout test coverage.
     10
     11        * http/tests/security/contentSecurityPolicy/1.1/frame-ancestors/frame-ancestors-overrides-X-Frames-Options-expected.txt: Added.
     12        * http/tests/security/contentSecurityPolicy/1.1/frame-ancestors/frame-ancestors-overrides-X-Frames-Options.html: Added.
     13        * http/tests/security/contentSecurityPolicy/resources/frame-ancestors-self-x-frame-options-deny.pl: Added.
     14
    1152019-04-24  chris fleizach  <cfleizach@apple.com>
    216

  • trunk/Source/WebCore/ChangeLog

    r244588 r244589  
     12019-04-24  Chris Dumez  <cdumez@apple.com>
     2
     3        X-Frame-Options header should be ignored when frame-ancestors CSP directive is present
     4        https://bugs.webkit.org/show_bug.cgi?id=197226
     5        <rdar://problem/50155649>
     6
     7        Reviewed by Alex Christensen.
     8
     9        X-Frame-Options header should be ignored when frame-ancestors CSP directive is present:
     10        - https://www.w3.org/TR/CSP3/#frame-ancestors-and-frame-options
     11
     12        Specification says:
     13        """
     14        In order to allow backwards-compatible deployment, the frame-ancestors directive _obsoletes_ the
     15        X-Frame-Options header. If a resource is delivered with an policy that includes a directive named
     16        frame-ancestors and whose disposition is "enforce", then the X-Frame-Options header MUST be ignored.
     17        """
     18
     19        Gecko and Blink follow the specification, WebKit does not. As a result, page [1] is broken with
     20        WebKit-only on Schwab.com. The page height is wrong and you cannot see all the ETFs as a result.
     21
     22        [1] https://www.schwab.com/public/schwab/investing/investment_help/investment_research/etf_research/etfs.html?&path=/Prospect/Research/etfs/overview/oneSourceETFs.asp
     23
     24        Test: http/tests/security/contentSecurityPolicy/1.1/frame-ancestors/frame-ancestors-overrides-X-Frames-Options.html
     25
     26        * loader/DocumentLoader.cpp:
     27        (WebCore::DocumentLoader::responseReceived):
     28        * page/csp/ContentSecurityPolicy.cpp:
     29        (WebCore::ContentSecurityPolicy::overridesXFrameOptions const):
     30        * page/csp/ContentSecurityPolicy.h:
     31        * page/csp/ContentSecurityPolicyDirectiveList.h:
     32        (WebCore::ContentSecurityPolicyDirectiveList::hasFrameAncestorsDirective const):
     33
    1342019-04-24  Zalan Bujtas  <zalan@apple.com>
    235

  • trunk/Source/WebCore/loader/DocumentLoader.cpp

    r244197 r244589  
    788788        }
    789789
    790         String frameOptions = response.httpHeaderFields().get(HTTPHeaderName::XFrameOptions);
    791         if (!frameOptions.isNull()) {
    792             if (frameLoader()->shouldInterruptLoadForXFrameOptions(frameOptions, url, identifier)) {
    793                 String message = "Refused to display '" + url.stringCenterEllipsizedToLength() + "' in a frame because it set 'X-Frame-Options' to '" + frameOptions + "'.";
    794                 m_frame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, message, identifier);
    795                 stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied(identifier, response);
    796                 return;
     790        if (!contentSecurityPolicy.overridesXFrameOptions()) {
     791            String frameOptions = response.httpHeaderFields().get(HTTPHeaderName::XFrameOptions);
     792            if (!frameOptions.isNull()) {
     793                if (frameLoader()->shouldInterruptLoadForXFrameOptions(frameOptions, url, identifier)) {
     794                    String message = "Refused to display '" + url.stringCenterEllipsizedToLength() + "' in a frame because it set 'X-Frame-Options' to '" + frameOptions + "'.";
     795                    m_frame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, message, identifier);
     796                    stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied(identifier, response);
     797                    return;
     798                }
    797799            }
    798800        }

  • trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp

    r244563 r244589  
    501501}
    502502
     503bool ContentSecurityPolicy::overridesXFrameOptions() const
     504{
     505    // If a resource is delivered with an policy that includes a directive named frame-ancestors and whose disposition
     506    // is "enforce", then the X-Frame-Options header MUST be ignored.
     507    // https://www.w3.org/TR/CSP3/#frame-ancestors-and-frame-options
     508    for (auto& policy : m_policies) {
     509        if (!policy->isReportOnly() && policy->hasFrameAncestorsDirective())
     510            return true;
     511    }
     512    return false;
     513}
     514
    503515bool ContentSecurityPolicy::allowFrameAncestors(const Vector<RefPtr<SecurityOrigin>>& ancestorOrigins, const URL& url, bool overrideContentSecurityPolicy) const
    504516{

  • trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h

    r244563 r244589  
    100100    bool allowFrameAncestors(const Frame&, const URL&, bool overrideContentSecurityPolicy = false) const;
    101101    WEBCORE_EXPORT bool allowFrameAncestors(const Vector<RefPtr<SecurityOrigin>>& ancestorOrigins, const URL&, bool overrideContentSecurityPolicy = false) const;
     102    WEBCORE_EXPORT bool overridesXFrameOptions() const;
    102103
    103104    enum class RedirectResponseReceived { No, Yes };

  • trunk/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.h

    r238771 r244589  
    7777
    7878    bool hasBlockAllMixedContentDirective() const { return m_hasBlockAllMixedContentDirective; }
     79    bool hasFrameAncestorsDirective() const { return !!m_frameAncestors; }
    7980
    8081    const String& evalDisabledErrorMessage() const { return m_evalDisabledErrorMessage; }

  • trunk/Source/WebKit/ChangeLog

    r244586 r244589  
     12019-04-24  Chris Dumez  <cdumez@apple.com>
     2
     3        X-Frame-Options header should be ignored when frame-ancestors CSP directive is present
     4        https://bugs.webkit.org/show_bug.cgi?id=197226
     5        <rdar://problem/50155649>
     6
     7        Reviewed by Alex Christensen.
     8
     9        * NetworkProcess/NetworkResourceLoader.cpp:
     10        (WebKit::NetworkResourceLoader::shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions):
     11
    1122019-04-24  Dean Jackson  <dino@apple.com>
    213

  • trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

    r244544 r244589  
    397397    if (!contentSecurityPolicy.allowFrameAncestors(m_parameters.frameAncestorOrigins, url))
    398398        return true;
    399     String xFrameOptions = m_response.httpHeaderField(HTTPHeaderName::XFrameOptions);
    400     if (!xFrameOptions.isNull() && shouldInterruptLoadForXFrameOptions(xFrameOptions, response.url())) {
    401         String errorMessage = "Refused to display '" + response.url().stringCenterEllipsizedToLength() + "' in a frame because it set 'X-Frame-Options' to '" + xFrameOptions + "'.";
    402         send(Messages::WebPage::AddConsoleMessage { m_parameters.webFrameID,  MessageSource::Security, MessageLevel::Error, errorMessage, identifier() }, m_parameters.webPageID);
    403         return true;
     399
     400    if (!contentSecurityPolicy.overridesXFrameOptions()) {
     401        String xFrameOptions = m_response.httpHeaderField(HTTPHeaderName::XFrameOptions);
     402        if (!xFrameOptions.isNull() && shouldInterruptLoadForXFrameOptions(xFrameOptions, response.url())) {
     403            String errorMessage = "Refused to display '" + response.url().stringCenterEllipsizedToLength() + "' in a frame because it set 'X-Frame-Options' to '" + xFrameOptions + "'.";
     404            send(Messages::WebPage::AddConsoleMessage { m_parameters.webFrameID,  MessageSource::Security, MessageLevel::Error, errorMessage, identifier() }, m_parameters.webPageID);
     405            return true;
     406        }
    404407    }
    405408    return false;
Note: See TracChangeset for help on using the changeset viewer.