From 6ba78c3b9c94b02614d206af8ceb370535e4de9c Mon Sep 17 00:00:00 2001 From: jww Date: Tue, 17 Nov 2015 16:12:29 -0800 Subject: [PATCH] Add Suborigin header and access control check for XHR This adds the plumbing for adding the Suborigin header on requests, and it particularly adds it for XHR preflights. This also adds tests to verify the behavior for XHR. BUG=336894 Review URL: https://codereview.chromium.org/1435123002 Cr-Commit-Position: refs/heads/master@{#360220} --- .../tests/security/resources/cors-script.php | 5 ++ ...rigin-cors-xhr-failure-output-expected.txt | 4 + .../suborigin-cors-xhr-failure-output.php | 33 ++++++++ .../suborigin-cors-xhr-preflight.php | 76 +++++++++++++++++++ .../core/fetch/CrossOriginAccessControl.cpp | 17 ++++- .../core/loader/DocumentThreadableLoader.cpp | 2 +- .../Source/core/loader/FrameFetchContext.cpp | 8 +- .../WebKit/Source/core/loader/FrameLoader.cpp | 4 +- .../core/xmlhttprequest/XMLHttpRequest.cpp | 2 +- .../platform/exported/WebURLRequest.cpp | 2 +- .../platform/network/ResourceRequest.cpp | 15 +++- .../Source/platform/network/ResourceRequest.h | 7 +- 12 files changed, 159 insertions(+), 16 deletions(-) create mode 100644 third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-failure-output-expected.txt create mode 100644 third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-failure-output.php create mode 100644 third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-preflight.php diff --git a/third_party/WebKit/LayoutTests/http/tests/security/resources/cors-script.php b/third_party/WebKit/LayoutTests/http/tests/security/resources/cors-script.php index fab2e078c59c0..80293ebbd8ec1 100644 --- a/third_party/WebKit/LayoutTests/http/tests/security/resources/cors-script.php +++ b/third_party/WebKit/LayoutTests/http/tests/security/resources/cors-script.php @@ -10,6 +10,11 @@ if (strtolower($_GET["credentials"]) == "true") { header("Access-Control-Allow-Credentials: true"); } + +if ($_SERVER["HTTP_SUBORIGIN"] == "foobar") { + header("Access-Control-Allow-Suborigin: foobar"); +} + header("Content-Type: application/javascript"); $delay = $_GET['delay']; if ($delay) diff --git a/third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-failure-output-expected.txt b/third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-failure-output-expected.txt new file mode 100644 index 0000000000000..89d54c99a52ea --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-failure-output-expected.txt @@ -0,0 +1,4 @@ +CONSOLE MESSAGE: line 9: If a Suborigin makes a request, a response without an Access-Control-Allow-Suborigin header should fail and should output a reasonable error message. +CONSOLE ERROR: XMLHttpRequest cannot load http://127.0.0.1:8000/security/resources/cors-script.php?cors=false. Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://foobar_127.0.0.1:8000' is therefore not allowed access. +ALERT: PASS: XHR correctly failed + diff --git a/third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-failure-output.php b/third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-failure-output.php new file mode 100644 index 0000000000000..f0b594b455e8c --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-failure-output.php @@ -0,0 +1,33 @@ + + + + + + + diff --git a/third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-preflight.php b/third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-preflight.php new file mode 100644 index 0000000000000..ee75787847a4c --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-xhr-preflight.php @@ -0,0 +1,76 @@ + + + + +Allow suborigin in HTTP header + + + + + +
+ + + diff --git a/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.cpp b/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.cpp index 825e90c8c663d..770adfbaec7bc 100644 --- a/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.cpp +++ b/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.cpp @@ -69,7 +69,7 @@ void updateRequestForAccessControl(ResourceRequest& request, SecurityOrigin* sec request.setFetchCredentialsMode(allowCredentials == AllowStoredCredentials ? WebURLRequest::FetchCredentialsModeInclude : WebURLRequest::FetchCredentialsModeOmit); if (securityOrigin) - request.setHTTPOrigin(securityOrigin->toAtomicString()); + request.setHTTPOrigin(securityOrigin); } ResourceRequest createAccessControlPreflightRequest(const ResourceRequest& request, SecurityOrigin* securityOrigin) @@ -133,6 +133,7 @@ bool passesAccessControlCheck(const ResourceResponse& response, StoredCredential { AtomicallyInitializedStaticReference(AtomicString, allowOriginHeaderName, (new AtomicString("access-control-allow-origin", AtomicString::ConstructFromLiteral))); AtomicallyInitializedStaticReference(AtomicString, allowCredentialsHeaderName, (new AtomicString("access-control-allow-credentials", AtomicString::ConstructFromLiteral))); + AtomicallyInitializedStaticReference(AtomicString, allowSuboriginHeaderName, (new AtomicString("access-control-allow-suborigin", AtomicString::ConstructFromLiteral))); int statusCode = response.httpStatusCode(); @@ -142,6 +143,18 @@ bool passesAccessControlCheck(const ResourceResponse& response, StoredCredential } const AtomicString& allowOriginHeaderValue = response.httpHeaderField(allowOriginHeaderName); + + // Check Suborigins, unless the Access-Control-Allow-Origin is '*', + // which implies that all Suborigins are okay as well. + if (securityOrigin->hasSuborigin() && allowOriginHeaderValue != starAtom) { + const AtomicString& allowSuboriginHeaderValue = response.httpHeaderField(allowSuboriginHeaderName); + AtomicString atomicSuboriginName(securityOrigin->suboriginName()); + if (allowSuboriginHeaderValue != starAtom && allowSuboriginHeaderValue != atomicSuboriginName) { + errorDescription = buildAccessControlFailureMessage("The 'Access-Control-Allow-Suborigin' header has a value '" + allowSuboriginHeaderValue + "' that is not equal to the supplied suborigin.", securityOrigin); + return false; + } + } + if (allowOriginHeaderValue == starAtom) { // A wildcard Access-Control-Allow-Origin can not be used if credentials are to be sent, // even with Access-Control-Allow-Credentials set to true. @@ -271,7 +284,7 @@ bool CrossOriginAccessControl::handleRedirect(SecurityOrigin* securityOrigin, Re if (redirectCrossOrigin) { // If now to a different origin, update/set Origin:. newRequest.clearHTTPOrigin(); - newRequest.setHTTPOrigin(securityOrigin->toAtomicString()); + newRequest.setHTTPOrigin(securityOrigin); // If the user didn't request credentials in the first place, update our // state so we neither request them nor expect they must be allowed. if (options.credentialsRequested == ClientDidNotRequestCredentials) diff --git a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp index cb7d2ed6b4d98..a8248c5440e4b 100644 --- a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp +++ b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp @@ -729,7 +729,7 @@ void DocumentThreadableLoader::loadActualRequest() OwnPtr actualOptions; actualOptions.swap(m_actualOptions); - actualRequest->setHTTPOrigin(securityOrigin()->toAtomicString()); + actualRequest->setHTTPOrigin(securityOrigin()); clearResource(); diff --git a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp index 0fe7ebba61bff..6084b70661771 100644 --- a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp +++ b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp @@ -97,16 +97,16 @@ void FrameFetchContext::addAdditionalRequestHeaders(ResourceRequest& request, Fe { bool isMainResource = type == FetchMainResource; if (!isMainResource) { - String outgoingOrigin; + RefPtr outgoingOrigin; if (!request.didSetHTTPReferrer()) { - outgoingOrigin = m_document->outgoingOrigin(); + outgoingOrigin = m_document->securityOrigin(); request.setHTTPReferrer(SecurityPolicy::generateReferrer(m_document->referrerPolicy(), request.url(), m_document->outgoingReferrer())); } else { RELEASE_ASSERT(SecurityPolicy::generateReferrer(request.referrerPolicy(), request.url(), request.httpReferrer()).referrer == request.httpReferrer()); - outgoingOrigin = SecurityOrigin::createFromString(request.httpReferrer())->toString(); + outgoingOrigin = SecurityOrigin::createFromString(request.httpReferrer()); } - request.addHTTPOriginIfNeeded(AtomicString(outgoingOrigin)); + request.addHTTPOriginIfNeeded(outgoingOrigin); } if (m_document) diff --git a/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/third_party/WebKit/Source/core/loader/FrameLoader.cpp index 13b25ebe95036..a39b9173054f9 100644 --- a/third_party/WebKit/Source/core/loader/FrameLoader.cpp +++ b/third_party/WebKit/Source/core/loader/FrameLoader.cpp @@ -123,7 +123,7 @@ ResourceRequest FrameLoader::resourceRequestFromHistoryItem(HistoryItem* item, request.setHTTPBody(formData); request.setHTTPContentType(item->formContentType()); RefPtr securityOrigin = SecurityOrigin::createFromString(item->referrer().referrer); - request.addHTTPOriginIfNeeded(securityOrigin->toAtomicString()); + request.addHTTPOriginIfNeeded(securityOrigin); } return request; } @@ -729,7 +729,7 @@ void FrameLoader::setReferrerForFrameRequest(ResourceRequest& request, ShouldSen request.setHTTPReferrer(referrer); RefPtr referrerOrigin = SecurityOrigin::createFromString(referrer.referrer); - request.addHTTPOriginIfNeeded(referrerOrigin->toAtomicString()); + request.addHTTPOriginIfNeeded(referrerOrigin); } FrameLoadType FrameLoader::determineFrameLoadType(const FrameLoadRequest& request) diff --git a/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp b/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp index bf7a846276288..14efa5d7009dc 100644 --- a/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp +++ b/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp @@ -906,7 +906,7 @@ void XMLHttpRequest::createRequest(PassRefPtr httpBody, Excepti request.addHTTPHeaderFields(m_requestHeaders); ThreadableLoaderOptions options; - options.preflightPolicy = uploadEvents ? ForcePreflight : ConsiderPreflight; + options.preflightPolicy = (uploadEvents || securityOrigin()->hasSuborigin()) ? ForcePreflight : ConsiderPreflight; options.crossOriginRequestPolicy = UseAccessControl; options.initiator = FetchInitiatorTypeNames::xmlhttprequest; options.contentSecurityPolicyEnforcement = ContentSecurityPolicy::shouldBypassMainWorld(&executionContext) ? DoNotEnforceContentSecurityPolicy : EnforceConnectSrcDirective; diff --git a/third_party/WebKit/Source/platform/exported/WebURLRequest.cpp b/third_party/WebKit/Source/platform/exported/WebURLRequest.cpp index 0cdec27cb9fcf..1b5f8a393a1a8 100644 --- a/third_party/WebKit/Source/platform/exported/WebURLRequest.cpp +++ b/third_party/WebKit/Source/platform/exported/WebURLRequest.cpp @@ -247,7 +247,7 @@ WebReferrerPolicy WebURLRequest::referrerPolicy() const void WebURLRequest::addHTTPOriginIfNeeded(const WebString& origin) { - m_private->m_resourceRequest->addHTTPOriginIfNeeded(origin); + m_private->m_resourceRequest->addHTTPOriginIfNeeded(WebSecurityOrigin::createFromString(origin)); } bool WebURLRequest::hasUserGesture() const diff --git a/third_party/WebKit/Source/platform/network/ResourceRequest.cpp b/third_party/WebKit/Source/platform/network/ResourceRequest.cpp index e0a0d44c4e0d2..5ee3653f09717 100644 --- a/third_party/WebKit/Source/platform/network/ResourceRequest.cpp +++ b/third_party/WebKit/Source/platform/network/ResourceRequest.cpp @@ -235,12 +235,20 @@ void ResourceRequest::clearHTTPReferrer() m_didSetHTTPReferrer = false; } +void ResourceRequest::setHTTPOrigin(PassRefPtr origin) +{ + setHTTPHeaderField("Origin", origin->toAtomicString()); + if (origin->hasSuborigin()) + setHTTPHeaderField("Suborigin", AtomicString(origin->suboriginName())); +} + void ResourceRequest::clearHTTPOrigin() { m_httpHeaderFields.remove("Origin"); + m_httpHeaderFields.remove("Suborigin"); } -void ResourceRequest::addHTTPOriginIfNeeded(const AtomicString& origin) +void ResourceRequest::addHTTPOriginIfNeeded(PassRefPtr origin) { if (!httpOrigin().isEmpty()) return; // Request already has an Origin header. @@ -257,10 +265,11 @@ void ResourceRequest::addHTTPOriginIfNeeded(const AtomicString& origin) // For non-GET and non-HEAD methods, always send an Origin header so the // server knows we support this feature. - if (origin.isEmpty()) { + AtomicString originString = origin->toAtomicString(); + if (originString.isEmpty()) { // If we don't know what origin header to attach, we attach the value // for an empty origin. - setHTTPOrigin(SecurityOrigin::createUnique()->toAtomicString()); + setHTTPOrigin(SecurityOrigin::createUnique()); return; } setHTTPOrigin(origin); diff --git a/third_party/WebKit/Source/platform/network/ResourceRequest.h b/third_party/WebKit/Source/platform/network/ResourceRequest.h index 1b30c2aba8b29..e4649d094a1a5 100644 --- a/third_party/WebKit/Source/platform/network/ResourceRequest.h +++ b/third_party/WebKit/Source/platform/network/ResourceRequest.h @@ -135,9 +135,12 @@ class PLATFORM_EXPORT ResourceRequest { void clearHTTPReferrer(); const AtomicString& httpOrigin() const { return httpHeaderField("Origin"); } - void setHTTPOrigin(const AtomicString& httpOrigin) { setHTTPHeaderField("Origin", httpOrigin); } + const AtomicString& httpSuborigin() const { return httpHeaderField("Suborigin"); } + // Note that these will also set and clear, respectively, the + // Suborigin header, if appropriate. + void setHTTPOrigin(PassRefPtr); void clearHTTPOrigin(); - void addHTTPOriginIfNeeded(const AtomicString& origin); + void addHTTPOriginIfNeeded(PassRefPtr); const AtomicString& httpUserAgent() const { return httpHeaderField("User-Agent"); } void setHTTPUserAgent(const AtomicString& httpUserAgent) { setHTTPHeaderField("User-Agent", httpUserAgent); }