Disable subresource redirect when restricted via CSP, CORS

This CL disables image redirect when content-security-policy disallows
loading image from other origins, to adhere to CSP restrictions.

This CL also disallows image redirect when crossorigin attribute is
specified.

Bug: 1098985
Change-Id: Ib6a25a31d716b6d0f318afd4e503482cede2148c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264588Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791272}

chrome/browser/data_saver/subresource_redirect_browsertest.cc

@@ -882,6 +882,124 @@ IN_PROC_BROWSER_TEST_F(SubresourceRedirectBrowserTest,
   VerifyImageCompressionPageInfoState(true);
 }
 
+// This test verifies images restricted via CSP img-src directive will not be
+// redirected.
+IN_PROC_BROWSER_TEST_F(SubresourceRedirectBrowserTest,
+                       DISABLE_ON_WIN_MAC_CHROMEOS(
+                           NoTriggerOnContentSecurityPolicyRestrictedImgSrc)) {
+  EnableDataSaver(true);
+  CreateUkmRecorder();
+  GURL url = HttpsURLWithPath("/load_image/image_csp_img_src.html");
+  SetUpPublicImageURLPaths(url, {"/load_image/image.png"});
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  content::FetchHistogramsFromChildProcesses();
+  metrics::SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
+
+  histogram_tester()->ExpectTotalCount(
+      "SubresourceRedirect.CompressionAttempt.ResponseCode", 0);
+
+  EXPECT_TRUE(RunScriptExtractBool("checkImage()"));
+
+  EXPECT_EQ(GURL(RunScriptExtractString("imageSrc()")).port(),
+            https_url().port());
+
+  VerifyIneligibleOtherImageUkm(1);
+  VerifyCompressibleImageUkm(0);
+  VerifyIneligibleImageHintsUnavailableUkm(0);
+  VerifyIneligibleMissingInImageHintsUkm(0);
+  VerifyImageCompressionPageInfoState(true);
+}
+
+// This test verifies images restricted via CSP img-src directive will not be
+// redirected.
+IN_PROC_BROWSER_TEST_F(
+    SubresourceRedirectBrowserTest,
+    DISABLE_ON_WIN_MAC_CHROMEOS(
+        NoTriggerOnContentSecurityPolicyRestrictedDefaultSrc)) {
+  EnableDataSaver(true);
+  CreateUkmRecorder();
+  GURL url = HttpsURLWithPath("/load_image/image_csp_default_src.html");
+  SetUpPublicImageURLPaths(url, {"/load_image/image.png"});
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  content::FetchHistogramsFromChildProcesses();
+  metrics::SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
+
+  histogram_tester()->ExpectTotalCount(
+      "SubresourceRedirect.CompressionAttempt.ResponseCode", 0);
+
+  EXPECT_TRUE(RunScriptExtractBool("checkImage()"));
+
+  EXPECT_EQ(GURL(RunScriptExtractString("imageSrc()")).port(),
+            https_url().port());
+
+  VerifyIneligibleOtherImageUkm(1);
+  VerifyCompressibleImageUkm(0);
+  VerifyIneligibleImageHintsUnavailableUkm(0);
+  VerifyIneligibleMissingInImageHintsUkm(0);
+  VerifyImageCompressionPageInfoState(true);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    SubresourceRedirectBrowserTest,
+    DISABLE_ON_WIN_MAC_CHROMEOS(
+        ImageRedirectedOnContentSecurityPolicyImgNotRestricted)) {
+  EnableDataSaver(true);
+  CreateUkmRecorder();
+
+  GURL url = HttpsURLWithPath("/load_image/image_csp_img_allowed.html");
+  SetUpPublicImageURLPaths(url, {"/load_image/image.png"});
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  RetryForHistogramUntilCountReached(
+      histogram_tester(), "SubresourceRedirect.CompressionAttempt.ResponseCode",
+      2);
+
+  histogram_tester()->ExpectBucketCount(
+      "SubresourceRedirect.CompressionAttempt.ResponseCode", net::HTTP_OK, 1);
+
+  histogram_tester()->ExpectBucketCount(
+      "SubresourceRedirect.CompressionAttempt.ResponseCode",
+      net::HTTP_TEMPORARY_REDIRECT, 1);
+
+  EXPECT_TRUE(RunScriptExtractBool("checkImage()"));
+  EXPECT_EQ(request_url().port(), compression_url().port());
+  VerifyCompressibleImageUkm(1);
+  VerifyIneligibleImageHintsUnavailableUkm(0);
+  VerifyIneligibleMissingInImageHintsUkm(0);
+  VerifyIneligibleOtherImageUkm(0);
+  VerifyImageCompressionPageInfoState(true);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    SubresourceRedirectBrowserTest,
+    DISABLE_ON_WIN_MAC_CHROMEOS(
+        TestOnlyImageWithoutCrossOriginAttributeIsRedirected)) {
+  EnableDataSaver(true);
+  CreateUkmRecorder();
+  GURL url = HttpsURLWithPath("/load_image/image_crossorigin_attribute.html");
+  SetUpPublicImageURLPaths(url, {"/load_image/image.png?nocrossorgin"});
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  RetryForHistogramUntilCountReached(
+      histogram_tester(), "SubresourceRedirect.CompressionAttempt.ResponseCode",
+      2);
+
+  histogram_tester()->ExpectBucketCount(
+      "SubresourceRedirect.CompressionAttempt.ResponseCode", net::HTTP_OK, 1);
+
+  EXPECT_TRUE(RunScriptExtractBool("checkAllImagesLoaded()"));
+  EXPECT_EQ(GURL(RunScriptExtractString("imageSrc()")).port(),
+            https_url().port());
+
+  VerifyCompressibleImageUkm(1);
+  VerifyIneligibleImageHintsUnavailableUkm(0);
+  VerifyIneligibleMissingInImageHintsUkm(0);
+  VerifyIneligibleOtherImageUkm(3);
+  VerifyImageCompressionPageInfoState(true);
+}
+
 // This test verifies that no image redirect happens when empty hints is sent.
 IN_PROC_BROWSER_TEST_F(
     SubresourceRedirectBrowserTest,

chrome/test/data/load_image/image_crossorigin_attribute.html

+<html>
+<head></head>
+<!--
+This page has four images to test the various values for crossorigin attribute.
+-->
+<img/>
+<img crossorigin="anonymous"/>
+<img crossorigin="use-credentials"/>
+<img crossorigin=""/>
+<script src="common.js"></script>
+<script>
+function checkAllImagesLoaded() {
+  return new Promise((resolve, reject) => {
+    Promise.all([
+      imageLoadedPromise(document.images[0]),
+      imageLoadedPromise(document.images[1]),
+      imageLoadedPromise(document.images[2]),
+      imageLoadedPromise(document.images[3])
+    ]).then(() => {
+        resolve(true);
+    });
+  });
+}
+
+window.onload = () => {
+  setTimeout(() => {
+    document.images[0].src = "image.png?nocrossorgin"
+    document.images[1].src = "image.png?crossorigin-anonymous"
+    document.images[2].src = "image.png?crossorigin-use-credentials"
+    document.images[3].src = "image.png?crossorigin-empty"
+  }, 1000)
+}
+</script>
+</html>

chrome/test/data/load_image/image_csp_default_src.html

+<html>
+<head>
+  <!--
+   Restrict all resource access for other origins and unblock inline script and
+   eval execution for testing purposes.
+  -->
+  <meta http-equiv="Content-Security-Policy"
+   content="default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'">
+</head>
+<img alt="long_placeholder_text"/>
+<script src="common.js"></script>
+<script>
+window.onload = () => {
+  setTimeout(() => {
+    document.images[0].src = "image.png"
+  }, 1000);
+}
+</script>
+</html>

chrome/test/data/load_image/image_csp_img_allowed.html

+<html>
+<head>
+  <!--
+   Restrict all resource access for other origins except images. Also unblock inline script
+   and eval execution for testing purposes.
+ -->
+  <meta http-equiv="Content-Security-Policy"
+   content="default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; img-src * ">
+</head>
+<img alt="long_placeholder_text"/>
+<script src="common.js"></script>
+<script>
+window.onload = () => {
+  setTimeout(() => {
+    document.images[0].src = "image.png"
+  }, 1000);
+}
+</script>
+</html>

chrome/test/data/load_image/image_csp_img_src.html

+<html>
+<head>
+  <meta http-equiv="Content-Security-Policy" content="img-src 'self'">
+</head>
+<img alt="long_placeholder_text"/>
+<script src="common.js"></script>
+<script>
+window.onload = () => {
+  setTimeout(() => {
+    document.images[0].src = "image.png"
+  }, 1000);
+}
+</script>
+</html>

third_party/blink/renderer/core/loader/image_loader.cc

@@ -118,6 +118,42 @@ bool CanReuseFromListOfAvailableImages(
   return true;
 }
 
+// Returns whether subresource redirect can be attempted for the image fetch.
+// Redirect to other origins could be disabled due to CSP or CORS restrictions.
+bool ShouldEnableSubresourceRedirect(HTMLImageElement* image_element,
+                                     const KURL& url) {
+  // Allow redirection only when DataSaver is enabled and subresource redirect
+  // feature is enabled which allows redirecting to better optimized versions.
+  if (!base::FeatureList::IsEnabled(blink::features::kSubresourceRedirect) ||
+      !GetNetworkStateNotifier().SaveDataEnabled()) {
+    return false;
+  }
+  // Enable subresource redirect only for <img> elements created by parser.
+  // Images created from javascript, fetched via XHR/Fetch API should not be
+  // subresource redirected due to the additional CORB/CORS handling needed for
+  // them.
+  if (!image_element || !image_element->ElementCreatedByParser()) {
+    return false;
+  }
+  // Create a cross origin URL by appending a string to the original host. This
+  // is used to find whether CSP is restricting image fetches from other
+  // origins.
+  KURL cross_origin_url = url;
+  cross_origin_url.SetHost(url.Host() + "crossorigin.com");
+  auto* content_security_policy =
+      image_element->GetExecutionContext()->GetContentSecurityPolicy();
+  if (content_security_policy &&
+      !content_security_policy->AllowImageFromSource(
+          cross_origin_url, cross_origin_url, RedirectStatus::kNoRedirect,
+          ReportingDisposition::kSuppressReporting)) {
+    return false;
+  }
+  // Allow subresource redirect only when cross-origin attribute is not set,
+  // which indicates CORS validation is not triggered for the image.
+  return (GetCrossOriginAttributeValue(image_element->FastGetAttribute(
+              html_names::kCrossoriginAttr)) == kCrossOriginAttributeNotSet);
+}
+
 }  // namespace
 
 class ImageLoader::Task {
@@ -555,21 +591,11 @@ void ImageLoader::DoUpdateFromElement(
       params.SetLazyImageNonBlocking();
     }
 
-    // Enable subresource redirect for <img> elements created by parser when
-    // data saver is on. Images created from javascript, fetched via XHR/Fetch
-    // API should not be subresource redirected due to the additional CORB/CORS
-    // handling needed for them.
-    // TODO(rajendrant): Disable subresource redirect when CORS,
-    // content-security-policy does not allow cross-origin accesses.
-    if (auto* html_image = DynamicTo<HTMLImageElement>(GetElement())) {
-      if (base::FeatureList::IsEnabled(blink::features::kSubresourceRedirect) &&
-          html_image->ElementCreatedByParser() &&
-          GetNetworkStateNotifier().SaveDataEnabled()) {
-        auto& resource_request = params.MutableResourceRequest();
-        resource_request.SetPreviewsState(
-            resource_request.GetPreviewsState() |
-            WebURLRequest::kSubresourceRedirectOn);
-      }
+    if (ShouldEnableSubresourceRedirect(
+            DynamicTo<HTMLImageElement>(GetElement()), params.Url())) {
+      auto& resource_request = params.MutableResourceRequest();
+      resource_request.SetPreviewsState(resource_request.GetPreviewsState() |
+                                        WebURLRequest::kSubresourceRedirectOn);
     }
 
     new_image_content = ImageResourceContent::Fetch(params, document.Fetcher());