CSP Suborigins

Source/core/fetch/CrossOriginAccessControl.cpp

 /*
  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 11 matching lines @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
  */
 
 #include "config.h"
 #include "core/fetch/CrossOriginAccessControl.h"
 
 #include "core/fetch/Resource.h"
 #include "core/fetch/ResourceLoaderOptions.h"
+#include "platform/RuntimeEnabledFeatures.h"
 #include "platform/network/HTTPParsers.h"
 #include "platform/network/ResourceRequest.h"
 #include "platform/network/ResourceResponse.h"
 #include "platform/weborigin/SchemeRegistry.h"
 #include "platform/weborigin/SecurityOrigin.h"
 #include "wtf/Threading.h"
 #include "wtf/text/AtomicString.h"
 #include "wtf/text/StringBuilder.h"
 
 namespace blink {
@@ skipping 55 matching lines @@
     }
 
     return preflightRequest;
 }
 
 static bool isOriginSeparator(UChar ch)
 {
     return isASCIISpace(ch) || ch == ',';
 }
 
+static bool experimentalContentSecurityPolicyFeaturesEnabled()
+{
+    return RuntimeEnabledFeatures::experimentalContentSecurityPolicyFeaturesEnabled();

[Mike West, 2014/10/23 12:59:19]

For clarity, I'd suggest adding a RuntimeEnabledFeature flag for suborigins.
It's only related to CSP insofar as CSP is the delivery mechanism; the feature
itself is distinct.

[jww, 2015/03/20 22:50:02]

Done.

+}
+
 static bool isInterestingStatusCode(int statusCode)
 {
     // Predicate that gates what status codes should be included in
     // console error messages for responses containing no access
     // control headers.
     return statusCode >= 400;
 }
 
 bool passesAccessControlCheck(const ResourceResponse& response, StoredCredentials includeCredentials, SecurityOrigin* securityOrigin, String& errorDescription)
 {
     AtomicallyInitializedStatic(AtomicString&, accessControlAllowOrigin = *new AtomicString("access-control-allow-origin", AtomicString::ConstructFromLiteral));
     AtomicallyInitializedStatic(AtomicString&, accessControlAllowCredentials = *new AtomicString("access-control-allow-credentials", AtomicString::ConstructFromLiteral));
+    AtomicallyInitializedStatic(AtomicString&, accessControlAllowFinerOrigin = *new AtomicString("access-control-allow-finer-origin", AtomicString::ConstructFromLiteral));

[Mike West, 2014/10/23 12:59:18]

"finer-origin"? Why not "suborigin"?

This is a spec question, obviously... not a question about this CL in
particular.

[jww, 2015/03/20 22:50:02]

I'll leave this for now since it's what I've said in all prior documents, but
let's bikeshed about this in spec-land ;-)

 
     if (!response.httpStatusCode()) {
         errorDescription = "Received an invalid response. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
         return false;
     }
 
     const AtomicString& accessControlOriginString = response.httpHeaderField(accessControlAllowOrigin);
-    if (accessControlOriginString == starAtom) {
+    const AtomicString& accessControlFinerOriginString = response.httpHeaderField(accessControlAllowFinerOrigin);
+    if (accessControlOriginString == starAtom || accessControlAllowFinerOrigin == starAtom) {

[Mike West, 2014/10/23 12:59:18]

This seems wrong: as written, either `a-c-allow-origin: *` OR
`a-c-allow-finer-origin: *` will return true for the access check. Shouldn't
both be required? (Again, a spec would be helpful: I may be misinterpreting the
intent entirely)

[jww, 2015/03/20 22:50:02]

This is on purpose, and I've tried to explain in the spec draft:
https://metromoxie.github.io/webappsec/specs/suborigins/index.html#cors

I'm second guessing it though. It seems reasonable to let * for all. Again,
discussion for spec-land, I think.

         // 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.
         if (includeCredentials == DoNotAllowStoredCredentials)
             return true;
         if (response.isHTTP()) {
-            errorDescription = "A wildcard '*' cannot be used in the 'Access-Control-Allow-Origin' header when the credentials flag is true. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
-            return false;
+            if (!experimentalContentSecurityPolicyFeaturesEnabled())
+                errorDescription = "A wildcard '*' cannot be used in the 'Access-Control-Allow-Origin' header when the credentials flag is true. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
+            else
+                errorDescription = "A wildcard '*' cannot be used in the 'Access-Control-Allow-Origin' or 'Access-Control-Allow-Finer-Origin' header when the credentials flag is true. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
         }
-    } else if (accessControlOriginString != securityOrigin->toAtomicString()) {
-        if (accessControlOriginString.isEmpty()) {
-            errorDescription = "No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
-
+    } else if (accessControlOriginString != securityOrigin->toAtomicString() && accessControlFinerOriginString != securityOrigin->toRawAtomicString()) {
+        if (accessControlOriginString.isEmpty() && accessControlFinerOriginString.isEmpty()) {
+            if (!experimentalContentSecurityPolicyFeaturesEnabled())
+                errorDescription = "No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
+            else
+                errorDescription = "No 'Access-Control-Allow-Origin' or 'Access-Control-Allow-accessControlFinerOriginString' header is present on the requested resource. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
             if (isInterestingStatusCode(response.httpStatusCode()))
                 errorDescription.append(" The response had HTTP status code " + String::number(response.httpStatusCode()) + ".");
-        } else if (accessControlOriginString.string().find(isOriginSeparator, 0) != kNotFound) {
-            errorDescription = "The 'Access-Control-Allow-Origin' header contains multiple values '" + accessControlOriginString + "', but only one is allowed. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
+        } else if (accessControlOriginString.string().find(isOriginSeparator, 0) != kNotFound || accessControlFinerOriginString.string().find(isOriginSeparator, 0) != kNotFound) {
+            if (!experimentalContentSecurityPolicyFeaturesEnabled())
+                errorDescription = "The 'Access-Control-Allow-Origin' header contains multiple values '" + accessControlOriginString + "', but only one is allowed. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
+            else
+                errorDescription = "The 'Access-Control-Allow-Origin' or 'Access-Control-Allow-Finer-Origin' header contains multiple values '" + accessControlOriginString + "', but only one is allowed. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
         } else {
             KURL headerOrigin(KURL(), accessControlOriginString);
-            if (!headerOrigin.isValid())
+            KURL headerFinerOrigin(KURL(), accessControlFinerOriginString);
+            if (!headerOrigin.isValid()) {
                 errorDescription = "The 'Access-Control-Allow-Origin' header contains the invalid value '" + accessControlOriginString + "'. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
-            else
-                errorDescription = "The 'Access-Control-Allow-Origin' header has a value '" + accessControlOriginString + "' that is not equal to the supplied origin. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
+            } else if (experimentalContentSecurityPolicyFeaturesEnabled() && !headerFinerOrigin.isValid()) {
+                errorDescription = "The 'Access-Control-Allow-Finer-Origin' header contains the invalid value '" + accessControlOriginString + "'. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
+            } else {
+                if (!experimentalContentSecurityPolicyFeaturesEnabled())
+                    errorDescription = "The 'Access-Control-Allow-Origin' header has a value '" + accessControlOriginString + "' that is not equal to the supplied origin. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
+                else
+                    errorDescription = "The 'Access-Control-Allow-Origin' or 'Access-Control-Allow-Finer-Origin' header has a value '" + accessControlOriginString + "' that is not equal to the supplied origin. Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";

[Mike West, 2014/10/23 12:59:19]

I'd suggest moving all the occurrences of these error strings out to a helper
function: something like String::format("The %s header has blah blah",
isEnabled() ? "a or b" : "a") in one place would be simpler to read and less
repetitive.

[jww, 2015/03/20 22:50:02]

Done. It's still a bit ugly, though, so any other ideas of beautifying it would
be welcome.

+            }
         }
         return false;
     }
 
     if (includeCredentials == AllowStoredCredentials) {
         const AtomicString& accessControlCredentialsString = response.httpHeaderField(accessControlAllowCredentials);
         if (accessControlCredentialsString != "true") {
             errorDescription = "Credentials flag is 'true', but the 'Access-Control-Allow-Credentials' header is '" + accessControlCredentialsString + "'. It must be 'true' to allow credentials.";
             return false;
         }
@@ skipping 80 matching lines @@
         request.setHTTPOrigin(securityOrigin->toAtomicString());
         // 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)
             options.allowCredentials = DoNotAllowStoredCredentials;
     }
     return true;
 }
 
 } // namespace blink