Do not block cross-site Flash requests in Site Isolation modes.

Flash requests can be distinguished by the lack of CORS, and they should
be allowed since Flash has its own cross-domain policy.  It is also
click-to-play, making this somewhat safer to allow.

TBR=nick@chromium.org
BUG=793953
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation

Change-Id: I0b191e0093cc5133a9d5421b1294e4bb91e64b6c
Reviewed-on: https://chromium-review.googlesource.com/817661
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523330}

content/browser/loader/cross_site_document_resource_handler.cc

@@ -98,8 +98,10 @@ class CrossSiteDocumentResourceHandler::OnWillReadController
 
 CrossSiteDocumentResourceHandler::CrossSiteDocumentResourceHandler(
     std::unique_ptr<ResourceHandler> next_handler,
-    net::URLRequest* request)
-    : LayeredResourceHandler(request, std::move(next_handler)) {}
+    net::URLRequest* request,
+    bool is_nocors_plugin_request)
+    : LayeredResourceHandler(request, std::move(next_handler)),
+      is_nocors_plugin_request_(is_nocors_plugin_request) {}
 
 CrossSiteDocumentResourceHandler::~CrossSiteDocumentResourceHandler() {}
 
@@ -374,15 +376,10 @@ bool CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders(
 
   // Give embedder a chance to skip document blocking for this response.
   if (GetContentClient()->browser()->ShouldBypassDocumentBlocking(
-          initiator, url, GetRequestInfo()->GetResourceType())) {
+          initiator, url, info->GetResourceType())) {
     return false;
   }
 
-  // TODO(creis): Don't block plugin requests with universal access. This could
-  // be done by allowing resource_type_ == RESOURCE_TYPE_PLUGIN_RESOURCE unless
-  // it had an Origin request header and IsValidCorsHeaderSet returned false.
-  // (That would matter for plugins without universal access, which use CORS.)
-
   // Allow the response through if it has valid CORS headers.
   std::string cors_header;
   response->head.headers->GetNormalizedHeader("access-control-allow-origin",
@@ -392,6 +389,16 @@ bool CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders(
     return false;
   }
 
+  // Don't block plugin requests with universal access (e.g., Flash).  Such
+  // requests are made without CORS, and thus dont have an Origin request
+  // header.  Other plugin requests (e.g., NaCl) are made using CORS and have an
+  // Origin request header.  If they fail the CORS check above, they should be
+  // blocked.
+  if (info->GetResourceType() == RESOURCE_TYPE_PLUGIN_RESOURCE &&
+      is_nocors_plugin_request_) {
+    return false;
+  }
+
   // We intend to block the response at this point.  However, we will usually
   // sniff the contents to confirm the MIME type, to avoid blocking incorrectly
   // labeled JavaScript, JSONP, etc files.

content/browser/loader/cross_site_document_resource_handler.h

@@ -63,7 +63,8 @@ class CONTENT_EXPORT CrossSiteDocumentResourceHandler
 
   CrossSiteDocumentResourceHandler(
       std::unique_ptr<ResourceHandler> next_handler,
-      net::URLRequest* request);
+      net::URLRequest* request,
+      bool is_nocors_plugin_request);
   ~CrossSiteDocumentResourceHandler() override;
 
   // LayeredResourceHandler overrides:
@@ -115,6 +116,13 @@ class CONTENT_EXPORT CrossSiteDocumentResourceHandler
   CrossSiteDocumentMimeType canonical_mime_type_ =
       CROSS_SITE_DOCUMENT_MIME_TYPE_OTHERS;
 
+  // Indicates whether this request was made by a plugin and was not using CORS.
+  // Such requests are exempt from blocking, while other plugin requests must be
+  // blocked if the CORS check fails.
+  // TODO(creis, nick): Replace this with a plugin process ID check to see if
+  // the plugin has universal access.
+  bool is_nocors_plugin_request_;
+
   // Tracks whether OnResponseStarted has been called, to ensure that it happens
   // before OnWillRead and OnReadCompleted.
   bool has_response_started_ = false;

content/browser/loader/cross_site_document_resource_handler_unittest.cc

@@ -237,6 +237,32 @@ const TestScenario kScenarios[] = {
         "<html><head>this should sniff as HTML",  // first_chunk
         Verdict::kAllowWithoutSniffing,           // verdict
     },
+    {
+        "Allowed: Cross-site fetch HTML from Flash without CORS", __LINE__,
+        "http://www.b.com/plugin.html",           // target_url
+        RESOURCE_TYPE_PLUGIN_RESOURCE,            // resource_type
+        "http://www.a.com/",                      // initiator_origin
+        OriginHeader::kOmit,                      // cors_request
+        "text/html",                              // response_mime_type
+        CROSS_SITE_DOCUMENT_MIME_TYPE_HTML,       // canonical_mime_type
+        false,                                    // include_no_sniff_header
+        AccessControlAllowOriginHeader::kOmit,    // cors_response
+        "<html><head>this should sniff as HTML",  // first_chunk
+        Verdict::kAllowWithoutSniffing,           // verdict
+    },
+    {
+        "Allowed: Cross-site fetch HTML from NaCl with CORS response", __LINE__,
+        "http://www.b.com/plugin.html",      // target_url
+        RESOURCE_TYPE_PLUGIN_RESOURCE,       // resource_type
+        "http://www.a.com/",                 // initiator_origin
+        OriginHeader::kInclude,              // cors_request
+        "text/html",                         // response_mime_type
+        CROSS_SITE_DOCUMENT_MIME_TYPE_HTML,  // canonical_mime_type
+        false,                               // include_no_sniff_header
+        AccessControlAllowOriginHeader::kAllowInitiatorOrigin,  // cors_response
+        "<html><head>this should sniff as HTML",                // first_chunk
+        Verdict::kAllowWithoutSniffing,                         // verdict
+    },
 
     // Allowed responses due to sniffing:
     {
@@ -417,6 +443,20 @@ const TestScenario kScenarios[] = {
         "<html><head>this should sniff as HTML",              // first_chunk
         Verdict::kBlockAfterSniffing,                         // verdict
     },
+    {
+        "Blocked: Cross-site fetch HTML from NaCl without CORS response",
+        __LINE__,
+        "http://www.b.com/plugin.html",           // target_url
+        RESOURCE_TYPE_PLUGIN_RESOURCE,            // resource_type
+        "http://www.a.com/",                      // initiator_origin
+        OriginHeader::kInclude,                   // cors_request
+        "text/html",                              // response_mime_type
+        CROSS_SITE_DOCUMENT_MIME_TYPE_HTML,       // canonical_mime_type
+        false,                                    // include_no_sniff_header
+        AccessControlAllowOriginHeader::kOmit,    // cors_response
+        "<html><head>this should sniff as HTML",  // first_chunk
+        Verdict::kBlockAfterSniffing,             // verdict
+    },
 };
 
 }  // namespace
@@ -440,12 +480,13 @@ class CrossSiteDocumentResourceHandlerTest
   // ResourceLoader.
   void Initialize(const std::string& target_url,
                   ResourceType resource_type,
-                  const std::string& initiator_origin) {
+                  const std::string& initiator_origin,
+                  OriginHeader cors_request) {
     stream_sink_status_ = net::URLRequestStatus::FromError(net::ERR_IO_PENDING);
 
     // Initialize |request_| from the parameters.
     request_ = context_.CreateRequest(GURL(target_url), net::DEFAULT_PRIORITY,
-                                      nullptr, TRAFFIC_ANNOTATION_FOR_TESTS);
+                                      &delegate_, TRAFFIC_ANNOTATION_FOR_TESTS);
     ResourceRequestInfo::AllocateForTesting(request_.get(), resource_type,
                                             nullptr,       // context
                                             3,             // render_process_id
@@ -464,8 +505,11 @@ class CrossSiteDocumentResourceHandlerTest
     stream_sink_ = stream_sink->GetWeakPtr();
 
     // Create the CrossSiteDocumentResourceHandler.
+    bool is_nocors_plugin_request =
+        resource_type == RESOURCE_TYPE_PLUGIN_RESOURCE &&
+        cors_request == OriginHeader::kOmit;
     document_blocker_ = std::make_unique<CrossSiteDocumentResourceHandler>(
-        std::move(stream_sink), request_.get());
+        std::move(stream_sink), request_.get(), is_nocors_plugin_request);
 
     // Create a mock loader to drive the CrossSiteDocumentResourceHandler.
     mock_loader_ =
@@ -511,6 +555,7 @@ class CrossSiteDocumentResourceHandlerTest
  protected:
   TestBrowserThreadBundle thread_bundle_;
   net::TestURLRequestContext context_;
+  net::TestDelegate delegate_;
   std::unique_ptr<net::URLRequest> request_;
 
   // |stream_sink_| is the handler that's immediately after |document_blocker_|
@@ -540,9 +585,8 @@ TEST_P(CrossSiteDocumentResourceHandlerTest, ResponseBlocking) {
   SCOPED_TRACE(testing::Message()
                << "\nScenario at " << __FILE__ << ":" << scenario.source_line);
 
-  // TODO(nick): Set origin header.
   Initialize(scenario.target_url, scenario.resource_type,
-             scenario.initiator_origin);
+             scenario.initiator_origin, scenario.cors_request);
   base::HistogramTester histograms;
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE,
@@ -717,9 +761,8 @@ TEST_P(CrossSiteDocumentResourceHandlerTest, OnWillReadDefer) {
   SCOPED_TRACE(testing::Message()
                << "\nScenario at " << __FILE__ << ":" << scenario.source_line);
 
-  // TODO(nick): Set origin header.
   Initialize(scenario.target_url, scenario.resource_type,
-             scenario.initiator_origin);
+             scenario.initiator_origin, scenario.cors_request);
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE,
             mock_loader_->OnWillStart(request_->url()));

content/browser/loader/resource_dispatcher_host_impl.cc

@@ -1449,7 +1449,7 @@ ResourceDispatcherHostImpl::CreateResourceHandler(
   }
 
   return AddStandardHandlers(request, request_data.resource_type,
-                             resource_context,
+                             resource_context, request_data.fetch_request_mode,
                              request_data.fetch_request_context_type,
                              request_data.fetch_mixed_content_context_type,
                              requester_info->appcache_service(), child_id,
@@ -1478,6 +1478,7 @@ ResourceDispatcherHostImpl::AddStandardHandlers(
     net::URLRequest* request,
     ResourceType resource_type,
     ResourceContext* resource_context,
+    network::mojom::FetchRequestMode fetch_request_mode,
     RequestContextType fetch_request_context_type,
     blink::WebMixedContentContextType fetch_mixed_content_context_type,
     AppCacheService* appcache_service,
@@ -1580,8 +1581,11 @@ ResourceDispatcherHostImpl::AddStandardHandlers(
     // Add a handler to block cross-site documents from the renderer process.
     // This should be pre mime-sniffing, since it affects whether the response
     // will be read, and since it looks at the original mime type.
-    handler.reset(
-        new CrossSiteDocumentResourceHandler(std::move(handler), request));
+    bool is_nocors_plugin_request =
+        resource_type == RESOURCE_TYPE_PLUGIN_RESOURCE &&
+        fetch_request_mode == network::mojom::FetchRequestMode::kNoCORS;
+    handler.reset(new CrossSiteDocumentResourceHandler(
+        std::move(handler), request, is_nocors_plugin_request));
   }
 
   return handler;
@@ -2152,10 +2156,12 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
           ->stream()
           ->CreateHandle();
 
+  // Safe to consider navigations as kNoCORS.
   // TODO(davidben): Fix the dependency on child_id/route_id. Those are used
   // by the ResourceScheduler. currently it's a no-op.
   handler = AddStandardHandlers(
       new_request.get(), resource_type, resource_context,
+      network::mojom::FetchRequestMode::kNoCORS,
       info.begin_params->request_context_type,
       info.begin_params->mixed_content_context_type,
       appcache_handle_core ? appcache_handle_core->GetAppCacheService()

content/browser/loader/resource_dispatcher_host_impl.h

@@ -639,6 +639,7 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
       net::URLRequest* request,
       ResourceType resource_type,
       ResourceContext* resource_context,
+      network::mojom::FetchRequestMode fetch_request_mode,
       RequestContextType fetch_request_context_type,
       blink::WebMixedContentContextType fetch_mixed_content_context_type,
       AppCacheService* appcache_service,