⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 246277 in webkit


Ignore:
Timestamp:
Jun 10, 2019, 1:30:50 PM (7 years ago)
Author:
dbates@webkit.org
Message:

[CSP] Blob URLs should inherit their CSP policy
​https://bugs.webkit.org/show_bug.cgi?id=198579
<rdar://problem/51366878>

Reviewed by Brent Fulgham.

Source/WebCore:

As per <​https://w3c.github.io/webappsec-csp/#security-inherit-csp> (Editor's Draft, 28 February 2019) blob
URLs should inherit their CSP policy from their parent (if they have one).

Test: http/tests/security/contentSecurityPolicy/navigate-self-to-blob.html

http/tests/security/contentSecurityPolicy/navigate-self-to-data-url.html

  • dom/Document.cpp:

(WebCore::Document::shouldInheritContentSecurityPolicyFromOwner const): Return true if the document's URL
is a Blob URL.
(WebCore::Document::initContentSecurityPolicy): Take a pointer to a ContentSecurityPolicy object that
represents the previous document's CSP. We only make us of this if the current URL is a Blob URL or a data
URL. Otherwise, do what we do now and take the policy from the owner frame.

  • dom/Document.h:
  • loader/DocumentWriter.cpp:

(WebCore::DocumentWriter::begin): Extend the lifetime of the previous document temporarily so that we can
pass its CSP to FrameLoader::didBeginDocument(). We need to do this extension because this function calls
FrameLoader::clear(), which can destroy the previous document and its ContentSecurityPolicy object. This
extension is also no different than if this function was called with a non-null ownerDocument except that
in that case it is the caller that extends the previous document's lifetime. Although it is tempting to
make use of ownerDocument to fix this bug by having the caller of begin() pass the previous document as
the ownerDocument when the new document's url (the one we are begin()ing) is a Blob URL. The ownerDocument
concept would privilege the Blob URL more than necessary; we only need to inherit the CSP policy from the
previous document for a Blob URL, not inherit the cookie URL or strict mixed content checking bit, etc.
We could make ContentSecurityPolicy ref-counted or even steal the ContentSecurityPolicy object from the
previous document. The latter is not of the question as a future enhancement, but the former seemed excessive
as a way to avoid extending the lifetime of the previous document because this would be the *only* call site
that actaully takes out a second ref of a ContentSecurityPolicy object. In general, shared ownership of
a ContentSecurityPolicy object does not make sense.

  • loader/FrameLoader.cpp:

(WebCore::FrameLoader::didBeginDocument): Pass the specified content security policy through to
Document::initContentSecurityPolicy().

  • loader/FrameLoader.h:

LayoutTests:

Add tests to ensure that a self navigation to a Blob or Data URL inherits its CSP policy from
its parent document.

  • http/tests/security/contentSecurityPolicy/navigate-self-to-blob-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/navigate-self-to-blob.html: Added.
  • http/tests/security/contentSecurityPolicy/navigate-self-to-data-url-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/navigate-self-to-data-url.html: Added.
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r246273 r246277  
     12019-06-10  Daniel Bates  <dabates@apple.com>
     2
     3        [CSP] Blob URLs should inherit their CSP policy
     4        https://bugs.webkit.org/show_bug.cgi?id=198579
     5        <rdar://problem/51366878>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        Add tests to ensure that a self navigation to a Blob or Data URL inherits its CSP policy from
     10        its parent document.
     11
     12        * http/tests/security/contentSecurityPolicy/navigate-self-to-blob-expected.txt: Added.
     13        * http/tests/security/contentSecurityPolicy/navigate-self-to-blob.html: Added.
     14        * http/tests/security/contentSecurityPolicy/navigate-self-to-data-url-expected.txt: Added.
     15        * http/tests/security/contentSecurityPolicy/navigate-self-to-data-url.html: Added.
     16
    1172019-06-10  Saam Barati  <sbarati@apple.com>
    218

  • trunk/Source/WebCore/ChangeLog

    r246273 r246277  
     12019-06-10  Daniel Bates  <dabates@apple.com>
     2
     3        [CSP] Blob URLs should inherit their CSP policy
     4        https://bugs.webkit.org/show_bug.cgi?id=198579
     5        <rdar://problem/51366878>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        As per <https://w3c.github.io/webappsec-csp/#security-inherit-csp> (Editor's Draft, 28 February 2019) blob
     10        URLs should inherit their CSP policy from their parent (if they have one).
     11
     12        Test: http/tests/security/contentSecurityPolicy/navigate-self-to-blob.html
     13              http/tests/security/contentSecurityPolicy/navigate-self-to-data-url.html
     14
     15        * dom/Document.cpp:
     16        (WebCore::Document::shouldInheritContentSecurityPolicyFromOwner const): Return true if the document's URL
     17        is a Blob URL.
     18        (WebCore::Document::initContentSecurityPolicy): Take a pointer to a ContentSecurityPolicy object that
     19        represents the previous document's CSP. We only make us of this if the current URL is a Blob URL or a data
     20        URL. Otherwise, do what we do now and take the policy from the owner frame.
     21        * dom/Document.h:
     22        * loader/DocumentWriter.cpp:
     23        (WebCore::DocumentWriter::begin): Extend the lifetime of the previous document temporarily so that we can
     24        pass its CSP to FrameLoader::didBeginDocument(). We need to do this extension because this function calls
     25        FrameLoader::clear(), which can destroy the previous document and its ContentSecurityPolicy object. This
     26        extension is also no different than if this function was called with a non-null ownerDocument except that
     27        in that case it is the caller that extends the previous document's lifetime. Although it is tempting to
     28        make use of ownerDocument to fix this bug by having the caller of begin() pass the previous document as
     29        the ownerDocument when the new document's url (the one we are begin()ing) is a Blob URL. The ownerDocument
     30        concept would privilege the Blob URL more than necessary; we only need to inherit the CSP policy from the
     31        previous document for a Blob URL, not inherit the cookie URL or strict mixed content checking bit, etc.
     32        We could make ContentSecurityPolicy ref-counted or even steal the ContentSecurityPolicy object from the
     33        previous document. The latter is not of the question as a future enhancement, but the former seemed excessive
     34        as a way to avoid extending the lifetime of the previous document because this would be the *only* call site
     35        that actaully takes out a second ref of a ContentSecurityPolicy object. In general, shared ownership of
     36        a ContentSecurityPolicy object does not make sense.
     37        * loader/FrameLoader.cpp:
     38        (WebCore::FrameLoader::didBeginDocument): Pass the specified content security policy through to
     39        Document::initContentSecurityPolicy().
     40        * loader/FrameLoader.h:
     41
    1422019-06-10  Saam Barati  <sbarati@apple.com>
    243

  • trunk/Source/WebCore/dom/Document.cpp

    r246267 r246277  
    59055905}
    59065906
    5907 bool Document::shouldInheritContentSecurityPolicyFromOwner() const
     5907// FIXME: The current criterion is stricter than <https://www.w3.org/TR/CSP3/#security-inherit-csp> (Editor's Draft, 28 February 2019).
     5908bool Document::shouldInheritContentSecurityPolicy() const
    59085909{
    59095910    ASSERT(m_frame);
    59105911    if (SecurityPolicy::shouldInheritSecurityOriginFromOwner(m_url))
    59115912        return true;
    5912     if (m_url.protocolIsData())
     5913    if (m_url.protocolIsData() || m_url.protocolIsBlob())
    59135914        return true;
    59145915    if (!isPluginDocument())
    … …  
    59225923}
    59235924
    5924 void Document::initContentSecurityPolicy()
     5925void Document::initContentSecurityPolicy(ContentSecurityPolicy* previousPolicy)
    59255926{
    59265927    // 1. Inherit Upgrade Insecure Requests
    … …  
    59305931
    59315932    // 2. Inherit Content Security Policy (without copying Upgrade Insecure Requests state).
    5932     if (!shouldInheritContentSecurityPolicyFromOwner())
    5933         return;
    5934     Frame* ownerFrame = parentFrame;
    5935     if (!ownerFrame)
    5936         ownerFrame = m_frame->loader().opener();
    5937     if (!ownerFrame)
    5938         return;
    5939     // FIXME: The CSP 3 spec. implies that only plugin documents delivered with a local scheme (e.g. blob, file, data)
    5940     // should inherit a policy.
     5933    if (!shouldInheritContentSecurityPolicy())
     5934        return;
     5935    ContentSecurityPolicy* ownerPolicy = nullptr;
     5936    if (previousPolicy && (m_url.protocolIsData() || m_url.protocolIsBlob()))
     5937        ownerPolicy = previousPolicy;
     5938    if (!ownerPolicy) {
     5939        Frame* ownerFrame = parentFrame;
     5940        if (!ownerFrame)
     5941            ownerFrame = m_frame->loader().opener();
     5942        if (ownerFrame)
     5943            ownerPolicy = ownerFrame->document()->contentSecurityPolicy();
     5944    }
     5945    if (!ownerPolicy)
     5946        return;
     5947    // FIXME: We are stricter than the CSP 3 spec. with regards to plugins: we prefer to inherit the full policy unless the plugin
     5948    // document is opened in a new window. The CSP 3 spec. implies that only plugin documents delivered with a local scheme (e.g. blob,
     5949    // file, data) should inherit a policy.
    59415950    if (isPluginDocument() && m_frame->loader().opener())
    5942         contentSecurityPolicy()->createPolicyForPluginDocumentFrom(*ownerFrame->document()->contentSecurityPolicy());
     5951        contentSecurityPolicy()->createPolicyForPluginDocumentFrom(*ownerPolicy);
    59435952    else
    5944         contentSecurityPolicy()->copyStateFrom(ownerFrame->document()->contentSecurityPolicy());
     5953        contentSecurityPolicy()->copyStateFrom(ownerPolicy);
    59455954}
    59465955

  • trunk/Source/WebCore/dom/Document.h

    r246231 r246277  
    11501150
    11511151    void initSecurityContext();
    1152     void initContentSecurityPolicy();
     1152    void initContentSecurityPolicy(ContentSecurityPolicy* previousPolicy);
    11531153
    11541154    void updateURLForPushOrReplaceState(const URL&);
    … …  
    15501550    friend class IgnoreDestructiveWriteCountIncrementer;
    15511551
    1552     bool shouldInheritContentSecurityPolicyFromOwner() const;
     1552    bool shouldInheritContentSecurityPolicy() const;
    15531553
    15541554    void updateTitleElement(Element& changingTitleElement);

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

    r241932 r246277  
    143143        document->createDOMWindow();
    144144
    145     // Per <http://www.w3.org/TR/upgrade-insecure-requests/>, we need to retain an ongoing set of upgraded
    146     // requests in new navigation contexts. Although this information is present when we construct the
    147     // Document object, it is discard in the subsequent 'clear' statements below. So, we must capture it
    148     // so we can restore it.
    149     HashSet<SecurityOriginData> insecureNavigationRequestsToUpgrade;
    150     if (auto* existingDocument = m_frame->document())
    151         insecureNavigationRequestsToUpgrade = existingDocument->contentSecurityPolicy()->takeNavigationRequestsToUpgrade();
    152    
     145    // Temporarily extend the lifetime of the existing document so that FrameLoader::clear() doesn't destroy it as
     146    // we need to retain its ongoing set of upgraded requests in new navigation contexts per <http://www.w3.org/TR/upgrade-insecure-requests/>
     147    // and we may also need to inherit its Content Security Policy in FrameLoader::didBeginDocument().
     148    RefPtr<Document> existingDocument = m_frame->document();
     149    auto* previousContentSecurityPolicy = existingDocument ? existingDocument->contentSecurityPolicy() : nullptr;
     150
    153151    m_frame->loader().clear(document.ptr(), !shouldReuseDefaultView, !shouldReuseDefaultView);
    154152    clear();
    … …  
    165163    m_frame->setDocument(document.copyRef());
    166164
    167     document->contentSecurityPolicy()->setInsecureNavigationRequestsToUpgrade(WTFMove(insecureNavigationRequestsToUpgrade));
     165    if (previousContentSecurityPolicy)
     166        document->contentSecurityPolicy()->setInsecureNavigationRequestsToUpgrade(previousContentSecurityPolicy->takeNavigationRequestsToUpgrade());
    168167
    169168    if (m_decoder)
    … …  
    175174    }
    176175
    177     m_frame->loader().didBeginDocument(dispatch);
     176    m_frame->loader().didBeginDocument(dispatch, previousContentSecurityPolicy);
    178177
    179178    document->implicitOpen();

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

    r246190 r246277  
    721721}
    722722
    723 void FrameLoader::didBeginDocument(bool dispatch)
     723void FrameLoader::didBeginDocument(bool dispatch, ContentSecurityPolicy* previousPolicy)
    724724{
    725725    m_needsClear = true;
    … …  
    737737
    738738    updateFirstPartyForCookies();
    739     m_frame.document()->initContentSecurityPolicy();
     739    m_frame.document()->initContentSecurityPolicy(previousPolicy);
    740740
    741741    const Settings& settings = m_frame.settings();

  • trunk/Source/WebCore/loader/FrameLoader.h

    r246190 r246277  
    231231
    232232    // Callbacks from DocumentWriter
    233     void didBeginDocument(bool dispatchWindowObjectAvailable);
     233    void didBeginDocument(bool dispatchWindowObjectAvailable, ContentSecurityPolicy* previousPolicy);
    234234
    235235    void receivedFirstData();
Note: See TracChangeset for help on using the changeset viewer.