Changeset 231263 in webkit


Ignore:
Timestamp:
May 2, 2018 2:13:28 PM (6 years ago)
Author:
youenn@apple.com
Message:

Use NetworkLoadChecker for navigation loads
​https://bugs.webkit.org/show_bug.cgi?id=184892
<rdar://problem/39652686>

Reviewed by Chris Dumez.

Source/WebCore:

Sanitize headers according response tainting.
If tainting is basic, it means same origin load in which case we only filter Cookie related headers.
If tainting is Opaque, we filter all uncommon headers.
If tainting is CORS, we filter all uncommon headers except the one explicitely allowed by CORS headers.
Covered by updated test.

  • platform/network/ResourceResponseBase.cpp:

(WebCore::ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting):
(WebCore::ResourceResponseBase::sanitizeHTTPHeaderFields):

  • platform/network/ResourceResponseBase.h:

Source/WebKit:

Compute whether a response is same origin in no-cors case.
This allows providing more precise filtering.
In case of navigate loads, set the tainting to basic which will make filtering to the minimum.

Pass the sourceOrigin for navigation loads as well.
Enable to restrict HTTP response access for navigation load.

Content Blockers are disabled for now in NetworkLoadChecker for navigation loads.
They should be reenabled as a follow-up.

Add a specific case to allow any redirection to about:// URLs.
While this does not conform with the spec, this keeps the existing WebKit behavior.

  • NetworkProcess/NetworkLoadChecker.cpp:

(WebKit::NetworkLoadChecker::NetworkLoadChecker):
(WebKit::NetworkLoadChecker::validateResponse):
(WebKit::NetworkLoadChecker::continueCheckingRequest):
(WebKit::NetworkLoadChecker::doesNotNeedCORSCheck const):

  • NetworkProcess/NetworkResourceLoader.cpp:

(WebKit::NetworkResourceLoader::sanitizeResponseIfPossible):

  • WebProcess/Network/WebLoaderStrategy.cpp:

(WebKit::WebLoaderStrategy::scheduleLoadFromNetworkProcess):
(WebKit::WebLoaderStrategy::isDoingLoadingSecurityChecks const):
We only do security checks if this runtime flag is on.

  • WebProcess/Network/WebLoaderStrategy.h:

LayoutTests:

Updated header-filtering.https.html to expect full headers except cookie-related for same origin loads.
Updated expected.txt files accordingly.

  • http/wpt/service-workers/header-filtering.https-expected.txt:
  • http/wpt/service-workers/header-filtering.https.html:
  • platform/mac/http/tests/webarchive/test-preload-resources-expected.txt:
Location:
trunk
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r231259 r231263  
     12018-05-02  Youenn Fablet  <youenn@apple.com>
     2
     3        Use NetworkLoadChecker for navigation loads
     4        https://bugs.webkit.org/show_bug.cgi?id=184892
     5        <rdar://problem/39652686>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Updated header-filtering.https.html to expect full headers except cookie-related for same origin loads.
     10        Updated expected.txt files accordingly.
     11
     12        * http/wpt/service-workers/header-filtering.https-expected.txt:
     13        * http/wpt/service-workers/header-filtering.https.html:
     14        * platform/mac/http/tests/webarchive/test-preload-resources-expected.txt:
     15
    1162018-05-02  Myles C. Maxfield  <mmaxfield@apple.com>
    217

  • trunk/LayoutTests/http/wpt/service-workers/header-filtering.https-expected.txt

    r230820 r231263  
    1 
    21
    32PASS Prepare tests: setup worker and register the client
    … …  
    98PASS Test no-cors script load
    109PASS Test cors script load
    11 FAIL Test HTML load assert_array_equals: lengths differ, expected 13 got 17
     10PASS Test HTML load
    1211PASS After tests clean-up
    1312

  • trunk/LayoutTests/http/wpt/service-workers/header-filtering.https.html

    r231255 r231263  
    101101    assert_array_equals(await data, ["Access-Control-Allow-Credentials","Access-Control-Allow-Methods","Access-Control-Allow-Origin",
    102102        "Access-Control-Expose-Headers","Cache-Control","Content-Length","Content-Type","Date","Referrer-Policy",
    103         "SourceMap","Timing-Allow-Origin","X-SourceMap","x-Header1"]);
     103        "SourceMap","Timing-Allow-Origin","X-SourceMap"]);
    104104}, "Test no-cors cross-origin fetch");
    105105
    … …  
    113113    assert_array_equals(await data, ["Access-Control-Allow-Credentials","Access-Control-Allow-Methods","Access-Control-Allow-Origin",
    114114        "Access-Control-Expose-Headers","Cache-Control","Content-Length","Content-Type","Date","Referrer-Policy",
    115         "SourceMap","Timing-Allow-Origin","X-SourceMap","x-Header1"]);
     115        "Server","SourceMap","Timing-Allow-Origin","X-SourceMap","x-header1","x-header2"]);
    116116}, "Test same-origin script load");
    117117
    … …  
    125125    assert_array_equals(await data, ["Access-Control-Allow-Credentials","Access-Control-Allow-Methods","Access-Control-Allow-Origin",
    126126        "Access-Control-Expose-Headers","Cache-Control","Content-Length","Content-Type","Date","Referrer-Policy",
    127         "SourceMap","Timing-Allow-Origin","X-SourceMap","x-Header1"]);
     127        "SourceMap","Timing-Allow-Origin","X-SourceMap"]);
    128128}, "Test no-cors script load");
    129129
    … …  
    149149    assert_array_equals(await data, ["Access-Control-Allow-Credentials","Access-Control-Allow-Methods","Access-Control-Allow-Origin",
    150150        "Access-Control-Expose-Headers","Cache-Control","Content-Length","Content-Type","Date","Referrer-Policy",
    151         "SourceMap","Timing-Allow-Origin","X-SourceMap","x-Header1"]);
     151        "Server", "SourceMap","Timing-Allow-Origin","X-SourceMap","x-header1", "x-header2"]);
    152152    frame.remove();
    153153}, "Test HTML load");

  • trunk/LayoutTests/platform/mac/http/tests/webarchive/test-preload-resources-expected.txt

    r230365 r231263  
    6666                                        <key>Last-Modified</key>
    6767                                        <string>Sun, 16 Nov 2008 16:55:00 GMT</string>
     68                                        <key>Server</key>
     69                                        <string>Apache/2.2.9 (Unix) mod_ssl/2.2.9 OpenSSL/0.9.7l PHP/5.2.6</string>
    6870                                </dict>
    6971                                <key>expectedContentLength</key>
    … …  
    101103                                        <key>Last-Modified</key>
    102104                                        <string>Sun, 16 Nov 2008 16:55:00 GMT</string>
     105                                        <key>Server</key>
     106                                        <string>Apache/2.2.9 (Unix) mod_ssl/2.2.9 OpenSSL/0.9.7l PHP/5.2.6</string>
    103107                                </dict>
    104108                                <key>expectedContentLength</key>
    … …  
    136140                                        <key>Last-Modified</key>
    137141                                        <string>Sun, 16 Nov 2008 16:55:00 GMT</string>
     142                                        <key>Server</key>
     143                                        <string>Apache/2.2.9 (Unix) mod_ssl/2.2.9 OpenSSL/0.9.7l PHP/5.2.6</string>
    138144                                </dict>
    139145                                <key>expectedContentLength</key>
    … …  
    171177                                        <key>Last-Modified</key>
    172178                                        <string>Sun, 16 Nov 2008 16:55:00 GMT</string>
     179                                        <key>Server</key>
     180                                        <string>Apache/2.2.9 (Unix) mod_ssl/2.2.9 OpenSSL/0.9.7l PHP/5.2.6</string>
    173181                                </dict>
    174182                                <key>expectedContentLength</key>
    … …  
    206214                                        <key>Last-Modified</key>
    207215                                        <string>Sun, 16 Nov 2008 16:55:00 GMT</string>
     216                                        <key>Server</key>
     217                                        <string>Apache/2.2.9 (Unix) mod_ssl/2.2.9 OpenSSL/0.9.7l PHP/5.2.6</string>
    208218                                </dict>
    209219                                <key>expectedContentLength</key>
    … …  
    241251                                        <key>Last-Modified</key>
    242252                                        <string>Sun, 16 Nov 2008 16:55:00 GMT</string>
     253                                        <key>Server</key>
     254                                        <string>Apache/2.2.9 (Unix) mod_ssl/2.2.9 OpenSSL/0.9.7l PHP/5.2.6</string>
    243255                                </dict>
    244256                                <key>expectedContentLength</key>
    … …  
    276288                                        <key>Last-Modified</key>
    277289                                        <string>Sun, 16 Nov 2008 16:55:00 GMT</string>
     290                                        <key>Server</key>
     291                                        <string>Apache/2.2.9 (Unix) mod_ssl/2.2.9 OpenSSL/0.9.7l PHP/5.2.6</string>
    278292                                </dict>
    279293                                <key>expectedContentLength</key>

  • trunk/Source/WebCore/ChangeLog

    r231259 r231263  
     12018-05-02  Youenn Fablet  <youenn@apple.com>
     2
     3        Use NetworkLoadChecker for navigation loads
     4        https://bugs.webkit.org/show_bug.cgi?id=184892
     5        <rdar://problem/39652686>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Sanitize headers according response tainting.
     10        If tainting is basic, it means same origin load in which case we only filter Cookie related headers.
     11        If tainting is Opaque, we filter all uncommon headers.
     12        If tainting is CORS, we filter all uncommon headers except the one explicitely allowed by CORS headers.
     13        Covered by updated test.
     14
     15        * platform/network/ResourceResponseBase.cpp:
     16        (WebCore::ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting):
     17        (WebCore::ResourceResponseBase::sanitizeHTTPHeaderFields):
     18        * platform/network/ResourceResponseBase.h:
     19
    1202018-05-02  Myles C. Maxfield  <mmaxfield@apple.com>
    221

  • trunk/Source/WebCore/platform/network/ResourceResponseBase.cpp

    r230365 r231263  
    390390}
    391391
    392 void ResourceResponseBase::sanitizeHTTPHeaderFields(SanitizationType type)
    393 {
    394     lazyInit(AllFields);
    395 
    396     m_httpHeaderFields.commonHeaders().remove(HTTPHeaderName::SetCookie);
    397     m_httpHeaderFields.commonHeaders().remove(HTTPHeaderName::SetCookie2);
    398 
    399     switch (type) {
    400     case SanitizationType::RemoveCookies:
     392void ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting()
     393{
     394    switch (m_tainting) {
     395    case ResourceResponse::Tainting::Basic:
    401396        return;
    402     case SanitizationType::Redirection: {
    403         auto commonHeaders = WTFMove(m_httpHeaderFields.commonHeaders());
    404         for (auto& header : commonHeaders) {
    405             if (isSafeRedirectionResponseHeader(header.key))
    406                 m_httpHeaderFields.add(header.key, WTFMove(header.value));
    407         }
    408         m_httpHeaderFields.uncommonHeaders().clear();
    409         return;
    410     }
    411     case SanitizationType::CrossOriginSafe: {
     397    case ResourceResponse::Tainting::Cors: {
    412398        HTTPHeaderMap filteredHeaders;
    413399        for (auto& header : m_httpHeaderFields.commonHeaders()) {
    … …  
    425411        }
    426412        m_httpHeaderFields = WTFMove(filteredHeaders);
    427     }
     413        return;
     414    }
     415    case ResourceResponse::Tainting::Opaque: {
     416        HTTPHeaderMap filteredHeaders;
     417        for (auto& header : m_httpHeaderFields.commonHeaders()) {
     418            if (isSafeCrossOriginResponseHeader(header.key))
     419                filteredHeaders.add(header.key, WTFMove(header.value));
     420        }
     421        m_httpHeaderFields = WTFMove(filteredHeaders);
     422        return;
     423    }
     424    case ResourceResponse::Tainting::Opaqueredirect: {
     425        auto location = httpHeaderField(HTTPHeaderName::Location);
     426        m_httpHeaderFields.clear();
     427        m_httpHeaderFields.add(HTTPHeaderName::Location, WTFMove(location));
     428    }
     429    }
     430}
     431
     432void ResourceResponseBase::sanitizeHTTPHeaderFields(SanitizationType type)
     433{
     434    lazyInit(AllFields);
     435
     436    m_httpHeaderFields.commonHeaders().remove(HTTPHeaderName::SetCookie);
     437    m_httpHeaderFields.commonHeaders().remove(HTTPHeaderName::SetCookie2);
     438
     439    switch (type) {
     440    case SanitizationType::RemoveCookies:
     441        return;
     442    case SanitizationType::Redirection: {
     443        auto commonHeaders = WTFMove(m_httpHeaderFields.commonHeaders());
     444        for (auto& header : commonHeaders) {
     445            if (isSafeRedirectionResponseHeader(header.key))
     446                m_httpHeaderFields.add(header.key, WTFMove(header.value));
     447        }
     448        m_httpHeaderFields.uncommonHeaders().clear();
     449        return;
     450    }
     451    case SanitizationType::CrossOriginSafe:
     452        sanitizeHTTPHeaderFieldsAccordingToTainting();
    428453    }
    429454}

  • trunk/Source/WebCore/platform/network/ResourceResponseBase.h

    r230365 r231263  
    200200    void parseCacheControlDirectives() const;
    201201    void updateHeaderParsedState(HTTPHeaderName);
     202    void sanitizeHTTPHeaderFieldsAccordingToTainting();
    202203
    203204protected:

  • trunk/Source/WebKit/ChangeLog

    r231257 r231263  
     12018-05-02  Youenn Fablet  <youenn@apple.com>
     2
     3        Use NetworkLoadChecker for navigation loads
     4        https://bugs.webkit.org/show_bug.cgi?id=184892
     5        <rdar://problem/39652686>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Compute whether a response is same origin in no-cors case.
     10        This allows providing more precise filtering.
     11        In case of navigate loads, set the tainting to basic which will make filtering to the minimum.
     12
     13        Pass the sourceOrigin for navigation loads as well.
     14        Enable to restrict HTTP response access for navigation load.
     15
     16        Content Blockers are disabled for now in NetworkLoadChecker for navigation loads.
     17        They should be reenabled as a follow-up.
     18
     19        Add a specific case to allow any redirection to about:// URLs.
     20        While this does not conform with the spec, this keeps the existing WebKit behavior.
     21
     22        * NetworkProcess/NetworkLoadChecker.cpp:
     23        (WebKit::NetworkLoadChecker::NetworkLoadChecker):
     24        (WebKit::NetworkLoadChecker::validateResponse):
     25        (WebKit::NetworkLoadChecker::continueCheckingRequest):
     26        (WebKit::NetworkLoadChecker::doesNotNeedCORSCheck const):
     27        * NetworkProcess/NetworkResourceLoader.cpp:
     28        (WebKit::NetworkResourceLoader::sanitizeResponseIfPossible):
     29        * WebProcess/Network/WebLoaderStrategy.cpp:
     30        (WebKit::WebLoaderStrategy::scheduleLoadFromNetworkProcess):
     31        (WebKit::WebLoaderStrategy::isDoingLoadingSecurityChecks const):
     32        We only do security checks if this runtime flag is on.
     33        * WebProcess/Network/WebLoaderStrategy.h:
     34
    1352018-05-02  Jer Noble  <jer.noble@apple.com>
    236

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

    r231107 r231263  
    4444using namespace WebCore;
    4545
     46static inline bool isSameOrigin(const URL& url, const SecurityOrigin* origin)
     47{
     48    return url.protocolIsData() || url.protocolIsBlob() || !origin || origin->canRequest(url);
     49}
     50
    4651NetworkLoadChecker::NetworkLoadChecker(FetchOptions&& options, PAL::SessionID sessionID, HTTPHeaderMap&& originalRequestHeaders, URL&& url, RefPtr<SecurityOrigin>&& sourceOrigin, PreflightPolicy preflightPolicy)
    4752    : m_options(WTFMove(options))
    … …  
    5257    , m_preflightPolicy(preflightPolicy)
    5358{
    54     if (m_options.mode == FetchOptions::Mode::Cors || m_options.mode == FetchOptions::Mode::SameOrigin)
    55         m_isSameOriginRequest = m_url.protocolIsData() || m_url.protocolIsBlob() || m_origin->canRequest(m_url);
     59    m_isSameOriginRequest = isSameOrigin(m_url, m_origin.get());
    5660    switch (options.credentials) {
    5761    case FetchOptions::Credentials::Include:
    … …  
    129133    }
    130134
    131     if (m_isSameOriginRequest) {
     135    if (m_options.mode == FetchOptions::Mode::Navigate || m_isSameOriginRequest) {
    132136        response.setTainting(ResourceResponse::Tainting::Basic);
    133137        return { };
    … …  
    188192    if (m_options.credentials == FetchOptions::Credentials::SameOrigin)
    189193        m_storedCredentialsPolicy = m_isSameOriginRequest && m_origin->canRequest(request.url()) ? StoredCredentialsPolicy::Use : StoredCredentialsPolicy::DoNotUse;
     194
     195    m_isSameOriginRequest = m_isSameOriginRequest && isSameOrigin(request.url(), m_origin.get());
    190196
    191197    if (doesNotNeedCORSCheck(request.url())) {
    … …  
    302308        return true;
    303309
    304     return m_isSameOriginRequest && m_origin->canRequest(url);
     310    return m_isSameOriginRequest;
    305311}
    306312
    … …  
    317323void NetworkLoadChecker::processContentExtensionRulesForLoad(ResourceRequest&& request, CompletionHandler<void(ResourceRequest&&, const ContentExtensions::BlockedStatus&)>&& callback)
    318324{
    319     if (!m_userContentControllerIdentifier) {
     325    // FIXME: Enable content blockers for navigation loads.
     326    if (!m_userContentControllerIdentifier || m_options.mode == FetchOptions::Mode::Navigate) {
    320327        ContentExtensions::BlockedStatus status;
    321328        callback(WTFMove(request), status);

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

    r231163 r231263  
    605605ResourceResponse NetworkResourceLoader::sanitizeResponseIfPossible(ResourceResponse&& response, ResourceResponse::SanitizationType type)
    606606{
    607     if (m_parameters.shouldRestrictHTTPResponseAccess) {
    608         if (type == ResourceResponse::SanitizationType::CrossOriginSafe) {
    609             // We reduce filtering when it would otherwise be visible to scripts.
    610             // FIXME: We should use response tainting once computed in Network Process.
    611             bool isSameOrigin = m_parameters.sourceOrigin ? m_parameters.sourceOrigin->canRequest(response.url()) : protocolHostAndPortAreEqual(response.url(), m_parameters.request.url());
    612             if (isSameOrigin && m_parameters.options.destination == FetchOptions::Destination::EmptyString)
    613                 type = ResourceResponse::SanitizationType::RemoveCookies;
    614         }
     607    if (m_parameters.shouldRestrictHTTPResponseAccess)
    615608        response.sanitizeHTTPHeaderFields(type);
    616     }
     609
    617610    return WTFMove(response);
    618611}
    … …  
    621614{
    622615    if (m_networkLoadChecker) {
    623         // FIXME: We should be doing this check when receiving the redirection.
    624         if (!newRequest.url().protocolIsInHTTPFamily() && m_redirectCount) {
     616        // FIXME: We should be doing this check when receiving the redirection and not allow about protocol as per fetch spec.
     617        if (!newRequest.url().protocolIsInHTTPFamily() && !newRequest.url().isBlankURL() && m_redirectCount) {
    625618            didFailLoading(ResourceError { String { }, 0, newRequest.url(), ASCIILiteral("Redirection to URL with a scheme that is not HTTP(S)"), ResourceError::Type::AccessControl });
    626619            return;

  • trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp

    r231026 r231263  
    302302#endif
    303303
     304    // FIXME: All loaders should provide their origin if navigation mode is cors/no-cors/same-origin.
     305    // As a temporary approach, we use the document origin if available or the HTTP Origin header otherwise.
     306    if (resourceLoader.isSubresourceLoader())
     307        loadParameters.sourceOrigin = static_cast<SubresourceLoader&>(resourceLoader).origin();
     308
     309    if (!loadParameters.sourceOrigin && document)
     310        loadParameters.sourceOrigin = &document->securityOrigin();
     311    if (!loadParameters.sourceOrigin) {
     312        auto origin = request.httpOrigin();
     313        if (!origin.isNull())
     314            loadParameters.sourceOrigin = SecurityOrigin::createFromString(origin);
     315    }
     316
    304317    if (loadParameters.options.mode != FetchOptions::Mode::Navigate) {
    305         // FIXME: All loaders should provide their origin if navigation mode is cors/no-cors/same-origin.
    306         // As a temporary approach, we use the document origin if available or the HTTP Origin header otherwise.
    307         if (resourceLoader.isSubresourceLoader())
    308             loadParameters.sourceOrigin = static_cast<SubresourceLoader&>(resourceLoader).origin();
    309 
    310         auto* document = resourceLoader.frame() ? resourceLoader.frame()->document() : nullptr;
    311         if (!loadParameters.sourceOrigin && document)
    312             loadParameters.sourceOrigin = &document->securityOrigin();
    313         if (!loadParameters.sourceOrigin) {
    314             auto origin = request.httpOrigin();
    315             if (!origin.isNull())
    316                 loadParameters.sourceOrigin = SecurityOrigin::createFromString(origin);
    317         }
    318318        ASSERT(loadParameters.sourceOrigin);
    319319        if (!loadParameters.sourceOrigin) {
    … …  
    323323    }
    324324
    325     // FIXME: We should also sanitize redirect response for navigations.
    326     loadParameters.shouldRestrictHTTPResponseAccess = RuntimeEnabledFeatures::sharedFeatures().restrictedHTTPResponseAccess() && resourceLoader.options().mode != FetchOptions::Mode::Navigate;
     325    loadParameters.shouldRestrictHTTPResponseAccess = RuntimeEnabledFeatures::sharedFeatures().restrictedHTTPResponseAccess();
    327326
    328327    loadParameters.isMainFrameNavigation = resourceLoader.frame() && resourceLoader.frame()->isMainFrame() && resourceLoader.options().mode == FetchOptions::Mode::Navigate;
    … …  
    664663}
    665664
     665bool WebLoaderStrategy::isDoingLoadingSecurityChecks() const
     666{
     667    return RuntimeEnabledFeatures::sharedFeatures().restrictedHTTPResponseAccess();
     668}
     669
    666670} // namespace WebKit

  • trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.h

    r230820 r231263  
    8484    void setOnLineState(bool);
    8585
    86     bool isDoingLoadingSecurityChecks() const final { return true; }
    87 
    8886private:
    8987    void scheduleLoad(WebCore::ResourceLoader&, WebCore::CachedResource*, bool shouldClearReferrerOnHTTPSToHTTPRedirect);
    … …  
    9593    WebCore::ResourceResponse responseFromResourceLoadIdentifier(uint64_t resourceLoadIdentifier) final;
    9694    WebCore::NetworkLoadMetrics networkMetricsFromResourceLoadIdentifier(uint64_t resourceLoadIdentifier) final;
     95
     96    bool isDoingLoadingSecurityChecks() const final;
    9797
    9898    HashSet<RefPtr<WebCore::ResourceLoader>> m_internallyFailedResourceLoaders;
Note: See TracChangeset for help on using the changeset viewer.