[OOR-CORS] Support --allow-file-access-from-files

network::ResourceRequest::request_initiator is set based on
blink::ResourceRequest::RequestorOrigin. The former is a url::Origin
the latter is a scoped_refptr<const blink::SecurityOrigin>.

A file: origin is handled in a special manner in blink. Without
--allow-file-access-from-files, it is NOT opaque but it is serialized
as "null". Currently conversion from blink::SecurityOrigin to
url::Origin relies on opacity (i.e., blink::SecurityOrigin::IsOpaque)
but that is not good for CORS.

This CL changes that. With this CL, A SecurityOrigin serializes as
"null" is converted to a unique url::Origin.

Bug: 736308
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I7b5ec4894c52499d85165689fd114e28bbd8bb97
Reviewed-on: https://chromium-review.googlesource.com/1131021Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594625}

content/browser/loader/cors_file_origin_browsertest.cc

@@ -263,17 +263,15 @@ IN_PROC_BROWSER_TEST_P(CORSFileOriginBrowserTestWithDisableWebSecurity,
   EXPECT_FALSE(is_preflight_requested());
 }
 
-// --allow-file-access-from-files is currently not supported by OOR-CORS.
-// We may remove the feature.
 INSTANTIATE_TEST_CASE_P(
     /* No test prefix */,
     CORSFileOriginBrowserTest,
-    ::testing::Values(false));
+    ::testing::Values(false, true));
 
 INSTANTIATE_TEST_CASE_P(
     /* No test prefix */,
     CORSFileOriginBrowserTestWithAllowFileAccessFromFiles,
-    ::testing::Values(false));
+    ::testing::Values(false, true));
 
 INSTANTIATE_TEST_CASE_P(
     /* No test prefix */,

content/renderer/loader/web_url_loader_impl.cc

@@ -644,10 +644,17 @@ void WebURLLoaderImpl::Context::Start(const WebURLRequest& request,
   resource_request->url = url_;
   resource_request->site_for_cookies = request.SiteForCookies();
   resource_request->upgrade_if_insecure = request.UpgradeIfInsecure();
-  resource_request->request_initiator =
-      request.RequestorOrigin().IsNull()
-          ? base::Optional<url::Origin>()
-          : base::Optional<url::Origin>(request.RequestorOrigin());
+  if (!request.RequestorOrigin().IsNull()) {
+    if (request.RequestorOrigin().ToString() == "null") {
+      // "file:" origin is treated like an opaque unique origin when
+      // allow-file-access-from-files is not specified. Such origin is not
+      // opaque (i.e., IsOpaque() returns false) but still serializes to
+      // "null".
+      resource_request->request_initiator = url::Origin();
+    } else {
+      resource_request->request_initiator = request.RequestorOrigin();
+    }
+  }
   resource_request->referrer = referrer_url;
 
   resource_request->referrer_policy =

services/network/cors/cors_url_loader.cc

@@ -190,7 +190,6 @@ void CORSURLLoader::OnReceiveResponse(
   DCHECK(!is_waiting_follow_redirect_call_);
 
   if (fetch_cors_flag_) {
-    // TODO(toyoshim): Reflect --allow-file-access-from-files flag.
     const auto error_status = CheckAccess(
         request_.url, response_head.headers->response_code(),
         GetHeaderString(response_head, header_names::kAccessControlAllowOrigin),
@@ -226,7 +225,6 @@ void CORSURLLoader::OnReceiveRedirect(
   // failure, then return a network error.
   if (fetch_cors_flag_ &&
       IsCORSEnabledRequestMode(request_.fetch_request_mode)) {
-    // TODO(toyoshim): Reflect --allow-file-access-from-files flag.
     const auto error_status = CheckAccess(
         request_.url, response_head.headers->response_code(),
         GetHeaderString(response_head, header_names::kAccessControlAllowOrigin),

services/network/cors/preflight_controller.cc

@@ -120,15 +120,13 @@ std::unique_ptr<PreflightResult> CreatePreflightResult(
     base::Optional<CORSErrorStatus>* detected_error_status) {
   DCHECK(detected_error_status);
 
-  // TODO(toyoshim): Reflect --allow-file-access-from-files flag.
   *detected_error_status = CheckPreflightAccess(
       final_url, head.headers->response_code(),
       GetHeaderString(head.headers, header_names::kAccessControlAllowOrigin),
       GetHeaderString(head.headers,
                       header_names::kAccessControlAllowCredentials),
       original_request.fetch_credentials_mode,
-      tainted ? url::Origin() : *original_request.request_initiator,
-      false /* allow_file_origin */);
+      tainted ? url::Origin() : *original_request.request_initiator);
   if (*detected_error_status)
     return nullptr;
 

services/network/public/cpp/cors/cors.cc

@@ -43,18 +43,6 @@ std::string ExtractMIMETypeFromMediaType(const std::string& media_type) {
   return std::string();
 }
 
-// url::Origin::Serialize() serializes all Origins with a 'file' scheme to
-// 'file://', but it isn't desirable for CORS check. Returns 'null' instead to
-// be aligned with HTTP Origin header calculation in Blink SecurityOrigin.
-// |allow_file_origin| is used to realize a behavior change that
-// the --allow-file-access-from-files command-line flag needs.
-// TODO(mkwst): Generalize and move to url/Origin.
-std::string Serialize(const url::Origin& origin, bool allow_file_origin) {
-  if (!allow_file_origin && origin.scheme() == url::kFileScheme)
-    return "null";
-  return origin.Serialize();
-}
-
 // Returns true only if |header_value| satisfies ABNF: 1*DIGIT [ "." 1*DIGIT ]
 bool IsSimilarToDoubleABNF(const std::string& header_value) {
   if (header_value.empty())
@@ -135,8 +123,7 @@ base::Optional<CORSErrorStatus> CheckAccess(
     const base::Optional<std::string>& allow_origin_header,
     const base::Optional<std::string>& allow_credentials_header,
     mojom::FetchCredentialsMode credentials_mode,
-    const url::Origin& origin,
-    bool allow_file_origin) {
+    const url::Origin& origin) {
   // TODO(toyoshim): This response status code check should not be needed. We
   // have another status code check after a CheckAccess() call if it is needed.
   if (!response_status_code)
@@ -159,7 +146,7 @@ base::Optional<CORSErrorStatus> CheckAccess(
       return CORSErrorStatus(mojom::CORSError::kWildcardOriginNotAllowed);
   } else if (!allow_origin_header) {
     return CORSErrorStatus(mojom::CORSError::kMissingAllowOriginHeader);
-  } else if (*allow_origin_header != Serialize(origin, allow_file_origin)) {
+  } else if (*allow_origin_header != origin.Serialize()) {
     // We do not use url::Origin::IsSameOriginWith() here for two reasons below.
     //  1. Allow "null" to match here. The latest spec does not have a clear
     //     information about this (https://fetch.spec.whatwg.org/#cors-check),
@@ -217,12 +204,10 @@ base::Optional<CORSErrorStatus> CheckPreflightAccess(
     const base::Optional<std::string>& allow_origin_header,
     const base::Optional<std::string>& allow_credentials_header,
     mojom::FetchCredentialsMode actual_credentials_mode,
-    const url::Origin& origin,
-    bool allow_file_origin) {
+    const url::Origin& origin) {
   const auto error_status =
       CheckAccess(response_url, response_status_code, allow_origin_header,
-                  allow_credentials_header, actual_credentials_mode, origin,
-                  allow_file_origin);
+                  allow_credentials_header, actual_credentials_mode, origin);
   if (!error_status)
     return base::nullopt;
 

services/network/public/cpp/cors/cors.h

@@ -56,8 +56,7 @@ base::Optional<CORSErrorStatus> CheckAccess(
     const base::Optional<std::string>& allow_origin_header,
     const base::Optional<std::string>& allow_credentials_header,
     mojom::FetchCredentialsMode credentials_mode,
-    const url::Origin& origin,
-    bool allow_file_origin = false);
+    const url::Origin& origin);
 
 // Performs a CORS access check on the CORS-preflight response parameters.
 // According to the note at https://fetch.spec.whatwg.org/#cors-preflight-fetch
@@ -70,8 +69,7 @@ base::Optional<CORSErrorStatus> CheckPreflightAccess(
     const base::Optional<std::string>& allow_origin_header,
     const base::Optional<std::string>& allow_credentials_header,
     mojom::FetchCredentialsMode actual_credentials_mode,
-    const url::Origin& origin,
-    bool allow_file_origin = false);
+    const url::Origin& origin);
 
 // Given a redirected-to URL, checks if the location is allowed
 // according to CORS. That is:

third_party/blink/renderer/platform/loader/cors/cors.cc

@@ -67,6 +67,14 @@ std::unique_ptr<net::HttpRequestHeaders> CreateNetHttpRequestHeaders(
   return request_headers;
 }
 
+url::Origin AsUrlOrigin(const SecurityOrigin& origin) {
+  // "file:" origin is treated like an opaque unique origin when
+  // allow-file-access-from-files is not specified. Such origin is not
+  // opaque (i.e., IsOpaque() returns false) but still serializes to
+  // "null".
+  return origin.ToString() == "null" ? url::Origin() : origin.ToUrlOrigin();
+}
+
 }  // namespace
 
 namespace CORS {
@@ -77,15 +85,12 @@ base::Optional<network::CORSErrorStatus> CheckAccess(
     const HTTPHeaderMap& response_header,
     network::mojom::FetchCredentialsMode credentials_mode,
     const SecurityOrigin& origin) {
-  std::unique_ptr<SecurityOrigin::PrivilegeData> privilege =
-      origin.CreatePrivilegeData();
   return network::cors::CheckAccess(
       response_url, response_status_code,
       GetHeaderValue(response_header, HTTPNames::Access_Control_Allow_Origin),
       GetHeaderValue(response_header,
                      HTTPNames::Access_Control_Allow_Credentials),
-      credentials_mode, origin.ToUrlOrigin(),
-      !privilege->block_local_access_from_local_origin_);
+      credentials_mode, AsUrlOrigin(origin));
 }
 
 base::Optional<network::CORSErrorStatus> CheckPreflightAccess(
@@ -94,15 +99,12 @@ base::Optional<network::CORSErrorStatus> CheckPreflightAccess(
     const HTTPHeaderMap& response_header,
     network::mojom::FetchCredentialsMode actual_credentials_mode,
     const SecurityOrigin& origin) {
-  std::unique_ptr<SecurityOrigin::PrivilegeData> privilege =
-      origin.CreatePrivilegeData();
   return network::cors::CheckPreflightAccess(
       response_url, response_status_code,
       GetHeaderValue(response_header, HTTPNames::Access_Control_Allow_Origin),
       GetHeaderValue(response_header,
                      HTTPNames::Access_Control_Allow_Credentials),
-      actual_credentials_mode, origin.ToUrlOrigin(),
-      !privilege->block_local_access_from_local_origin_);
+      actual_credentials_mode, AsUrlOrigin(origin));
 }
 
 base::Optional<network::CORSErrorStatus> CheckRedirectLocation(
@@ -112,7 +114,7 @@ base::Optional<network::CORSErrorStatus> CheckRedirectLocation(
     CORSFlag cors_flag) {
   base::Optional<url::Origin> origin_to_pass;
   if (origin)
-    origin_to_pass = origin->ToUrlOrigin();
+    origin_to_pass = AsUrlOrigin(*origin);
 
   // Blink-side implementations rewrite the origin instead of setting the
   // tainted flag.
@@ -198,7 +200,7 @@ network::mojom::FetchResponseType CalculateResponseTainting(
     CORSFlag cors_flag) {
   base::Optional<url::Origin> origin_to_pass;
   if (origin)
-    origin_to_pass = origin->ToUrlOrigin();
+    origin_to_pass = AsUrlOrigin(*origin);
   return network::cors::CalculateResponseTainting(
       url, request_mode, origin_to_pass, cors_flag == CORSFlag::Set);
 }