Disallow cross-origin subresources from asking for credentials
<rdar://problem/36162271>
Created attachment 333316 [details] Patch and layout tests
Created attachment 333318 [details] Patch and layout tests
Comment on attachment 333318 [details] Patch and layout tests View in context: https://bugs.webkit.org/attachment.cgi?id=333318&action=review > LayoutTests/http/tests/security/mixedContent/secure-redirect-to-secure-redirect-to-basic-auth-secure-image.https-expected.txt:7 > -PASS did load image. > +FAIL did not load image. This is expected since the image is cross origin with the origin of the page and hence it will be blocked from loading. Will change test to print PASS instead of FAIL.
Created attachment 333321 [details] Patch and layout tests
Comment on attachment 333321 [details] Patch and layout tests Attachment 333321 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6405534 New failing tests: http/tests/security/mixedContent/secure-redirect-to-secure-redirect-to-basic-auth-secure-image.https.html
Created attachment 333336 [details] Archive of layout-test-results from ews101 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 333337 [details] Patch and layout tests
Comment on attachment 333337 [details] Patch and layout tests Attachment 333337 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/6407424 New failing tests: http/tests/security/basic-auth-subresource.html http/tests/security/credentials-iframes-allowCrossOriginSubresourcesToAskForCredentials.html
Created attachment 333350 [details] Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 333396 [details] Patch and layout tests Add Windows-specific results for http/tests/security/basic-auth-subresource.html due to <https://bugs.webkit.org/show_bug.cgi?id=182609>. Skip allowCrossOriginSubresourcesToAskForCredentials-suffixed tests on Windows since DumpRenderTree does not support parsing test options. See <https://bugs.webkit.org/show_bug.cgi?id=17328> for more details.
(In reply to Daniel Bates from comment #11) > [...] See <https://bugs.webkit.org/show_bug.cgi?id=17328> for more details. *bug #173281
Created attachment 333398 [details] Patch and layout tests
Comment on attachment 333398 [details] Patch and layout tests View in context: https://bugs.webkit.org/attachment.cgi?id=333398&action=review > Source/WebCore/ChangeLog:18 > + Before landing I will add a remark of the form: Since r224134 WebKit has disallowed mixed content subresources from asking for credentials. Let's go further and forbid all cross-origin subresources from asking for credentials.
Comment on attachment 333398 [details] Patch and layout tests View in context: https://bugs.webkit.org/attachment.cgi?id=333398&action=review > Source/WebCore/loader/ResourceLoader.h:190 > + bool canRequestAskUserForCredentials() const; Can this just be called canAskUserForCredentials()? The first time I read this, I interpreted request as a verb and got confused. I get that we're basing the decision on the request URL, but maybe that fact doesn't need to be in the function signature. r=me either way.
(In reply to Andy Estes from comment #15) > Comment on attachment 333398 [details] > Patch and layout tests > > View in context: > https://bugs.webkit.org/attachment.cgi?id=333398&action=review > > > Source/WebCore/loader/ResourceLoader.h:190 > > + bool canRequestAskUserForCredentials() const; > > Can this just be called canAskUserForCredentials()? The first time I read > this, I interpreted request as a verb and got confused. I get that we're > basing the decision on the request URL, but maybe that fact doesn't need to > be in the function signature. r=me either way. I was planning to rename ResourceLoader::isAllowedToAskUserForCredentials() to canAskUserForCredentials() in a subsequent patch. I hope you do not mind that I rename canRequestAskUserForCredentials() to shouldBlockCrossOriginAuthenticationChallenge() before landing.
(In reply to Daniel Bates from comment #16) > (In reply to Andy Estes from comment #15) > > Comment on attachment 333398 [details] > > Patch and layout tests > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=333398&action=review > > > > > Source/WebCore/loader/ResourceLoader.h:190 > > > + bool canRequestAskUserForCredentials() const; > > > > Can this just be called canAskUserForCredentials()? The first time I read > > this, I interpreted request as a verb and got confused. I get that we're > > basing the decision on the request URL, but maybe that fact doesn't need to > > be in the function signature. r=me either way. > > I was planning to rename ResourceLoader::isAllowedToAskUserForCredentials() > to canAskUserForCredentials() in a subsequent patch. I hope you do not mind > that I rename canRequestAskUserForCredentials() to > shouldBlockCrossOriginAuthenticationChallenge() before landing. Or maybe shouldAllowResourceToAskForCredentials?
Committed r228486: <https://trac.webkit.org/changeset/228486>