Relax same-origin policy for ServiceWorker openWindow() in Chromium.

content/browser/service_worker/service_worker_version.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/service_worker/service_worker_version.h"
 
 #include "base/command_line.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/message_port_message_filter.h"
 #include "content/browser/message_port_service.h"
 #include "content/browser/service_worker/embedded_worker_instance.h"
 #include "content/browser/service_worker/embedded_worker_registry.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_utils.h"
 #include "content/browser/storage_partition_impl.h"
 #include "content/common/service_worker/service_worker_messages.h"
@@ skipping 245 matching lines @@
       url, Referrer::SanitizeForRequest(
                url, Referrer(script_url, blink::WebReferrerPolicyDefault)),
       NEW_FOREGROUND_TAB, ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
       true /* is_renderer_initiated */);
 
   GetContentClient()->browser()->OpenURL(
       browser_context, params,
       base::Bind(&DidOpenURL, callback));
 }
 
-void KillEmbeddedWorkerProcess(int process_id, ResultCode code) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-  RenderProcessHost* render_process_host =
-      RenderProcessHost::FromID(process_id);
-  if (render_process_host->GetHandle() != base::kNullProcessHandle)
-    render_process_host->ReceivedBadMessage();
-}
-
 }  // namespace
 
 ServiceWorkerVersion::ServiceWorkerVersion(
     ServiceWorkerRegistration* registration,
     const GURL& script_url,
     int64 version_id,
     base::WeakPtr<ServiceWorkerContextCore> context)
     : version_id_(version_id),
       registration_id_(kInvalidServiceWorkerVersionId),
       script_url_(script_url),
@@ skipping 864 matching lines @@
   scoped_refptr<ServiceWorkerVersion> protect(this);
   callback->Run(SERVICE_WORKER_OK, accept_connection);
   RemoveCallbackAndStopIfDoomed(&cross_origin_connect_callbacks_, request_id);
 }
 
 void ServiceWorkerVersion::OnOpenWindow(int request_id, const GURL& url) {
   // Just abort if we are shutting down.
   if (!context_)
     return;
 
-  if (url.GetOrigin() != script_url_.GetOrigin()) {
-    // There should be a same origin check by Blink, if the request is still not
-    // same origin, the process might be compromised and should be eliminated.
-    DVLOG(1) << "Received a cross origin openWindow() request from a service "
-                "worker. Killing associated process.";
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            base::Bind(&KillEmbeddedWorkerProcess,
-                                       embedded_worker_->process_id(),
-                                       RESULT_CODE_KILLED_BAD_MESSAGE));
-    return;
+  GURL sanitized_url = url;
+
+  // Blink consider all about: scheme URLs as about:blank. We need to sanitize
+  // them accordingly to prevent CanRequestURL() call below to fail on them.
+  if (sanitized_url.SchemeIs(url::kAboutScheme))
+    sanitized_url = GURL(url::kAboutBlankURL);
+
+  // This call will check whether the process should be able to access the given
+  // URL. It is possible to receive requests to open such URLs because the

[falken, 2015/03/06 16:03:08]

nit: "receive requests to open disallowed URLs"?

+  // renderer side checks are slightly different. For example, view-source
+  // scheme will not be filtered out by Blink.
+  if (!ChildProcessSecurityPolicyImpl::GetInstance()->CanRequestURL(
+          embedded_worker_->process_id(), sanitized_url)) {
+    sanitized_url = GURL(url::kAboutBlankURL);

[falken, 2015/03/06 16:03:08]

Maybe I'm missing something... why open about:blank instead of rejecting
openWindow() with an error for such URLs?

[mlamouri (slow - plz ping), 2015/03/06 16:17:19]

Hmm, actually, I should have left a comment about that. I have a few thoughts.

Ideally, I would like to kill the process but I can't because it wasn't very
hard to find cases (view-source://) where the request might go trough.

Also, I could use RenderProcessHost::FilterURL() which is doing a couple more
checks than what I do (and sets to about:blank too).

I have no strong opinion between rejecting and allowing the call with
about:blank. I ended up allowing the call to be closer to the behaviour of
window.open() which fails for chrome:// URLs or file://. openWindow() will only
reject for situations where window.open() would throw and will open about:blank
when window.open() would.

WDYT?

[mlamouri (slow - plz ping), 2015/03/06 16:17:19]

Hmm, actually, I should have left a comment about that. I have a few thoughts.

Ideally, I would like to kill the process but I can't because it wasn't very
hard to find cases (view-source://) where the request might go trough.

Also, I could use RenderProcessHost::FilterURL() which is doing a couple more
checks than what I do (and sets to about:blank too).

I have no strong opinion between rejecting and allowing the call with
about:blank. I ended up allowing the call to be closer to the behaviour of
window.open() which fails for chrome:// URLs or file://. openWindow() will only
reject for situations where window.open() would throw and will open about:blank
when window.open() would.

WDYT?

[mlamouri (slow - plz ping), 2015/03/06 16:17:19]

Hmm, actually, I should have left a comment about that. I have a few thoughts.

Ideally, I would like to kill the process but I can't because it wasn't very
hard to find cases (view-source://) where the request might go trough.

Also, I could use RenderProcessHost::FilterURL() which is doing a couple more
checks than what I do (and sets to about:blank too).

I have no strong opinion between rejecting and allowing the call with
about:blank. I ended up allowing the call to be closer to the behaviour of
window.open() which fails for chrome:// URLs or file://. openWindow() will only
reject for situations where window.open() would throw and will open about:blank
when window.open() would.

WDYT?

[falken, 2015/03/06 16:43:32]

Ah that's interesting, I didn't know window.open('view-source://blahblah');
opens about:blank. I was originally thinking it'd be weird for the developer to
not get an indication that the URL wasn't really opened, but don't feel
strongly. Following window.open sounds OK. I agree a comment to that effect
would be nice.

   }
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&OpenWindowOnUI,
-                 url,
+                 sanitized_url,
                  script_url_,
                  embedded_worker_->process_id(),
                  make_scoped_refptr(context_->wrapper()),
                  base::Bind(&ServiceWorkerVersion::DidOpenWindow,
                             weak_factory_.GetWeakPtr(),
                             request_id)));
 }
 
 void ServiceWorkerVersion::DidOpenWindow(int request_id,
                                          int render_process_id,
@@ skipping 386 matching lines @@
     int request_id) {
   callbacks->Remove(request_id);
   if (is_doomed_) {
     // The stop should be already scheduled, but try to stop immediately, in
     // order to release worker resources soon.
     StopWorkerIfIdle();
   }
 }
 
 }  // namespace content