Skip to content

Commit e295238

Browse files
achristensen07achristensen07
committed
Main resource requests need cachePartition
https://bugs.webkit.org/show_bug.cgi?id=168806 Source/WebCore: <rdar://30639764> Reviewed by Brady Eidson. Test: http/tests/security/credentials-main-resource.html r211751 caused an unintended regression on pages whose main resource is protected by basic authentication. We were not setting the cache partition for main resource requests, and we use the cache partition now for credentials, so the credentials for the main resource were not being put into a partition in the CredentialStorage that would not be used for subresources of the page, whose requests had the correct partition for the domain of the page. This caused users to have to enter their credentials twice, once for the main resource and once for any subresources. This is fixed by using the domain from the main resource request as the cache partition. Elsewhere the Document is used to get the cache partition, but there is no Document yet when requesting the main resource. * loader/DocumentLoader.cpp: (WebCore::DocumentLoader::startLoadingMainResource): Set the cache partition for the main resource loads based on the SecurityOrigin of the initial request if we are loading the main resource for a new top document. If the main resource request is redirected, then we will still use the partition of the initial request because that is what the user requested and that is where the user entered the credentials. * loader/cache/CachedResourceLoader.h: * loader/cache/CachedResourceRequest.cpp: (WebCore::CachedResourceRequest::setDomainForCachePartition): * loader/cache/CachedResourceRequest.h: Source/WebKit2: Reviewed by Brady Eidson. * NetworkProcess/NetworkResourceLoader.cpp: (WebKit::NetworkResourceLoader::continueWillSendRequest): LayoutTests: Reviewed by Brady Eidson. * http/tests/security/credentials-main-resource-expected.txt: Added. * http/tests/security/credentials-main-resource.html: Added. * http/tests/security/resources/credentials-main-resource.php: Added. Canonical link: https://commits.webkit.org/185957@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@213126 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 9706121 commit e295238

11 files changed

Lines changed: 106 additions & 2 deletions

File tree

‎LayoutTests/ChangeLog‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
2017-02-28 Alex Christensen <achristensen@webkit.org>
2+
3+
Main resource requests need cachePartition
4+
https://bugs.webkit.org/show_bug.cgi?id=168806
5+
6+
Reviewed by Brady Eidson.
7+
8+
* http/tests/security/credentials-main-resource-expected.txt: Added.
9+
* http/tests/security/credentials-main-resource.html: Added.
10+
* http/tests/security/resources/credentials-main-resource.php: Added.
11+
112
2017-02-28 Alex Christensen <achristensen@webkit.org>
213

314
REGRESSION: LayoutTest http/tests/security/credentials-iframes.html is failing on ios-simulator

‎LayoutTests/http/tests/security/credentials-main-resource-expected.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
ALERT: Authenticated as user: testuser password: testpass
2+
Main Resource Credentials: testuser, testpass
3+

‎LayoutTests/http/tests/security/credentials-main-resource.html‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<script>
2+
if (window.testRunner) {
3+
testRunner.dumpAsText();
4+
testRunner.waitUntilDone();
5+
internals.settings.setStorageBlockingPolicy('BlockThirdParty');
6+
}
7+
window.location = "http://testuser:testpass@127.0.0.1:8000/security/resources/credentials-main-resource.php";
8+
</script>

‎LayoutTests/http/tests/security/resources/credentials-main-resource.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
if (!isset($_SERVER['PHP_AUTH_USER'])) {
3+
header('WWW-Authenticate: Basic realm="WebKit test - credentials-in-main-resource"');
4+
header('HTTP/1.0 401 Unauthorized');
5+
exit;
6+
}
7+
echo "Main Resource Credentials: " . $_SERVER['PHP_AUTH_USER'] . ", " . $_SERVER['PHP_AUTH_PW'] . "<br/>";
8+
?>
9+
<script>
10+
11+
if (window.internals)
12+
internals.settings.setStorageBlockingPolicy('BlockThirdParty');
13+
14+
var request = new XMLHttpRequest();
15+
request.onreadystatechange = function () {
16+
if (this.readyState == 4) {
17+
alert(this.responseText);
18+
if (window.testRunner)
19+
testRunner.notifyDone();
20+
}
21+
};
22+
request.open('GET', 'http://127.0.0.1:8000/security/resources/basic-auth.php?username=testuser&password=testpass', true);
23+
request.send(null);
24+
</script>

‎Source/WebCore/ChangeLog‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,34 @@
1+
2017-02-28 Alex Christensen <achristensen@webkit.org>
2+
3+
Main resource requests need cachePartition
4+
https://bugs.webkit.org/show_bug.cgi?id=168806
5+
<rdar://30639764>
6+
7+
Reviewed by Brady Eidson.
8+
9+
Test: http/tests/security/credentials-main-resource.html
10+
11+
r211751 caused an unintended regression on pages whose main resource is protected
12+
by basic authentication. We were not setting the cache partition for main resource
13+
requests, and we use the cache partition now for credentials, so the credentials for
14+
the main resource were not being put into a partition in the CredentialStorage that
15+
would not be used for subresources of the page, whose requests had the correct partition
16+
for the domain of the page. This caused users to have to enter their credentials twice,
17+
once for the main resource and once for any subresources. This is fixed by using the
18+
domain from the main resource request as the cache partition. Elsewhere the Document is
19+
used to get the cache partition, but there is no Document yet when requesting the main resource.
20+
21+
* loader/DocumentLoader.cpp:
22+
(WebCore::DocumentLoader::startLoadingMainResource):
23+
Set the cache partition for the main resource loads based on the SecurityOrigin of the
24+
initial request if we are loading the main resource for a new top document. If the main resource
25+
request is redirected, then we will still use the partition of the initial request because that is
26+
what the user requested and that is where the user entered the credentials.
27+
* loader/cache/CachedResourceLoader.h:
28+
* loader/cache/CachedResourceRequest.cpp:
29+
(WebCore::CachedResourceRequest::setDomainForCachePartition):
30+
* loader/cache/CachedResourceRequest.h:
31+
132
2017-02-28 Alex Christensen <achristensen@webkit.org>
233

334
REGRESSION: LayoutTest http/tests/security/credentials-iframes.html is failing on ios-simulator

‎Source/WebCore/loader/DocumentLoader.cpp‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1479,7 +1479,16 @@ void DocumentLoader::startLoadingMainResource()
14791479
RELEASE_LOG_IF_ALLOWED("startLoadingMainResource: Starting load (frame = %p, main = %d)", m_frame, m_frame->isMainFrame());
14801480

14811481
static NeverDestroyed<ResourceLoaderOptions> mainResourceLoadOptions(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials, ClientCredentialPolicy::MayAskClientForCredentials, FetchOptions::Credentials::Include, SkipSecurityCheck, FetchOptions::Mode::NoCors, IncludeCertificateInfo, ContentSecurityPolicyImposition::SkipPolicyCheck, DefersLoadingPolicy::AllowDefersLoading, CachingPolicy::AllowCaching);
1482-
m_mainResource = m_cachedResourceLoader->requestMainResource(CachedResourceRequest(ResourceRequest(request), mainResourceLoadOptions));
1482+
CachedResourceRequest mainResourceRequest(ResourceRequest(request), mainResourceLoadOptions);
1483+
if (!m_frame->isMainFrame() && m_frame->document()) {
1484+
// If we are loading the main resource of a subframe, use the cache partition of the main document.
1485+
mainResourceRequest.setDomainForCachePartition(*m_frame->document());
1486+
} else {
1487+
auto origin = SecurityOrigin::create(request.url());
1488+
origin->setStorageBlockingPolicy(frameLoader()->frame().settings().storageBlockingPolicy());
1489+
mainResourceRequest.setDomainForCachePartition(origin->domainForCachePartition());
1490+
}
1491+
m_mainResource = m_cachedResourceLoader->requestMainResource(WTFMove(mainResourceRequest));
14831492

14841493
#if ENABLE(CONTENT_EXTENSIONS)
14851494
if (m_mainResource && m_mainResource->errorOccurred() && m_frame->page() && m_mainResource->resourceError().domain() == ContentExtensions::WebKitContentBlockerDomain) {

‎Source/WebCore/loader/cache/CachedResourceLoader.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ class Document;
5353
class DocumentLoader;
5454
class Frame;
5555
class ImageLoader;
56+
class Settings;
5657
class URL;
5758

5859
// The CachedResourceLoader provides a per-context interface to the MemoryCache

‎Source/WebCore/loader/cache/CachedResourceRequest.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ void CachedResourceRequest::setDomainForCachePartition(Document& document)
134134
m_resourceRequest.setDomainForCachePartition(document.topOrigin().domainForCachePartition());
135135
}
136136

137+
void CachedResourceRequest::setDomainForCachePartition(const String& domain)
138+
{
139+
m_resourceRequest.setDomainForCachePartition(domain);
140+
}
141+
137142
static inline String acceptHeaderValueFromType(CachedResource::Type type)
138143
{
139144
switch (type) {

‎Source/WebCore/loader/cache/CachedResourceRequest.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ class CachedResourceRequest {
7676
void applyBlockedStatus(const ContentExtensions::BlockedStatus&);
7777
#endif
7878
void setDomainForCachePartition(Document&);
79+
void setDomainForCachePartition(const String&);
7980
bool isLinkPreload() const { return m_isLinkPreload; }
8081
void setIsLinkPreload() { m_isLinkPreload = true; }
8182

‎Source/WebKit2/ChangeLog‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
2017-02-28 Alex Christensen <achristensen@webkit.org>
2+
3+
Main resource requests need cachePartition
4+
https://bugs.webkit.org/show_bug.cgi?id=168806
5+
6+
Reviewed by Brady Eidson.
7+
8+
* NetworkProcess/NetworkResourceLoader.cpp:
9+
(WebKit::NetworkResourceLoader::continueWillSendRequest):
10+
111
2017-02-27 Alex Christensen <achristensen@webkit.org>
212

313
Begin enabling WebRTC on 64-bit

0 commit comments

Comments
 (0)