Enforce mime-type check when loading Origin Policy manifest.

This CL modifies OriginPolicyFetcher to enforce that Origin Policy
manifests load with the mimetype

application/originpolicy+json

If an incorrect mime type is received, loading is aborted, and
the OriginPolicy object is returned with an empty policy and
status kNoPolicyApplies. Since this same pathway is hit if the
manifest load returns 404, we can also set kNoPolicyApplies in that
case as well, and a test is included for that.

The test includes a valid CSP which, if applied, would lead the test
to fail as eval() would not succeed. If the manifest is rejected due
to its incorrect mimetype, then the test passes.

Bug: 1051169
Change-Id: I59e5bde20c8a21a4fae8dbc3c6f58f34cb292195
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2080696Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748364}

chrome/test/origin_policy/origin_policy_browsertest.cc

@@ -119,12 +119,6 @@ IN_PROC_BROWSER_TEST_F(OriginPolicyBrowserTest, ApplyPolicy) {
             NavigateToAndReturnTitle("/page-with-policy.html"));
 }
 
-IN_PROC_BROWSER_TEST_F(OriginPolicyBrowserTest, ErrorPolicy404) {
-  SetStatus(net::HTTP_NOT_FOUND);
-  EXPECT_EQ(base::ASCIIToUTF16(kErrorInterstitialTitle),
-            NavigateToAndReturnTitle("/page-with-policy.html"));
-}
-
 IN_PROC_BROWSER_TEST_F(OriginPolicyBrowserTest, ErrorPolicy301Redirect) {
   SetStatus(net::HTTP_MOVED_PERMANENTLY);
   SetLocationHeader("/.well-known/origin-policy/example-policy");

services/network/origin_policy/origin_policy_fetcher.cc

@@ -58,6 +58,9 @@ void OriginPolicyFetcher::FetchPolicy(mojom::URLLoaderFactory* factory) {
 
   FetchCallback done = base::BindOnce(&OriginPolicyFetcher::OnPolicyHasArrived,
                                       base::Unretained(this));
+  SimpleURLLoader::OnResponseStartedCallback check_content_type =
+      base::BindOnce(&OriginPolicyFetcher::OnResponseStarted,
+                     base::Unretained(this));
 
   // Create and configure the SimpleURLLoader for the policy.
   std::unique_ptr<ResourceRequest> policy_request =
@@ -70,11 +73,34 @@ void OriginPolicyFetcher::FetchPolicy(mojom::URLLoaderFactory* factory) {
   url_loader_ =
       SimpleURLLoader::Create(std::move(policy_request), traffic_annotation);
 
+  url_loader_->SetOnResponseStartedCallback(std::move(check_content_type));
   // Start the download, and pass the callback for when we're finished.
   url_loader_->DownloadToString(factory, std::move(done),
                                 kOriginPolicyMaxPolicySize);
 }
 
+void OriginPolicyFetcher::OnResponseStarted(
+    const GURL& final_url,
+    const mojom::URLResponseHead& response_head) {
+  std::string mime_type;
+  // If the manifest's return code isn't success (2xx) or if the manifest's
+  // mimetype is incorrect, reject it and return with kNoPolicyApplies.
+  if (!response_head.headers ||
+      response_head.headers->response_code() / 100 != 2 ||
+      (response_head.headers->GetMimeType(&mime_type) &&
+       mime_type != "application/originpolicy+json")) {
+    // The manifest file returned with incorrect content-type, or unsuccessful
+    // return code, so bail out.
+    OriginPolicy result;
+    result.state = OriginPolicyState::kNoPolicyApplies;
+    // Do not add code after this call as it will destroy this object.
+    // When |this| is destroyed, |url_loader_| will be release, so when this
+    // callback returns the SimpleURLLoader will bail out gracefully.
+    // This also means that OnPolicyHasArrived() below will not be called.
+    owner_policy_manager_->FetcherDone(this, result, std::move(callback_));
+  }
+}
+
 void OriginPolicyFetcher::OnPolicyHasArrived(
     std::unique_ptr<std::string> policy_content) {
   OriginPolicy result;

services/network/origin_policy/origin_policy_fetcher.h

@@ -39,6 +39,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) OriginPolicyFetcher {
  private:
   using FetchCallback = base::OnceCallback<void(std::unique_ptr<std::string>)>;
 
+  void OnResponseStarted(const GURL& final_url,
+                         const mojom::URLResponseHead& response_head);
   void OnPolicyHasArrived(std::unique_ptr<std::string> policy_content);
   void FetchPolicy(mojom::URLLoaderFactory* factory);
 

third_party/blink/web_tests/external/wpt/.well-known/origin-policy

@@ -5,6 +5,20 @@ import glob
 def main(request, response):
     host_piece = request.url_parts.hostname.split(".")[0]
 
+    # Default return code for manifests if found.
+    return_code = 200
+
+    # Reserve 'op99' for tests that should use return code 404.
+    if host_piece == "op99":
+        return_code = 404
+
+    # Default mime type for returned data.
+    content_type = "application/originpolicy+json"
+
+    # Reserve 'op100' for testing incorrect mime type for manifest file.
+    if host_piece == "op100":
+        content_type = "text/plain"
+
     filepath_pattern = os.path.normpath(os.path.join(os.path.dirname(os.path.abspath(
         __file__)), "../origin-policy/policies/", "{} *.json".format(host_piece)))
 
@@ -15,4 +29,4 @@ def main(request, response):
 
     with open(matches[0]) as f:
         data = f.read()
-        return 200, [('Content-Type', 'application/originpolicy+json')], data
+        return return_code, [('Content-Type', content_type)], data

third_party/blink/web_tests/external/wpt/origin-policy/content-security/manifest-404.https.html

+<!DOCTYPE HTML>
+<meta charset="utf-8">
+<title>Missing manifest file.</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../resources/origin-policy-test-runner.js"></script>
+
+<div id="log"></div>
+
+<script>
+"use strict";
+<!-- Manifests delivered with 404 status code must be ignored. -->
+runTestsInSubframe({
+  hostname: "op99",
+  testJS: "resources/allow-unsafe-eval.mjs"
+});
+</script>

third_party/blink/web_tests/external/wpt/origin-policy/content-security/manifest-mimetype.https.html

+<!DOCTYPE HTML>
+<meta charset="utf-8">
+<title>Valid "content_security/policies" member must be ignored</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../resources/origin-policy-test-runner.js"></script>
+
+<div id="log"></div>
+
+<script>
+"use strict";
+runTestsInSubframe({
+  hostname: "op100",
+  testJS: "resources/allow-unsafe-eval.mjs"
+});
+</script>

third_party/blink/web_tests/external/wpt/origin-policy/policies/op100 manifest-mimetype.json

+{
+  "ids" : ["csp-valid-but-served-with-invalid-mimetype"],
+  "content_security" : {
+      "policies" : ["script-src 'self' 'unsafe-inline'"]
+  }
+}

third_party/blink/web_tests/external/wpt/origin-policy/policies/op99 csp-valid-manifest-with-404.json

+{
+  "ids" : ["csp-valid-but-served-as-404"],
+  "content_security" : {
+      "policies" : ["script-src 'self' 'unsafe-inline'"]
+  }
+}