Add URL origin checks for Service Worker (un)registration

Now the browser will register or unregister a Service Worker only if document url,
scope, and script url have the same origin.

This patch removes TODOs for adding SW to ChildProcessSecurityPolicy, since it
turns out the only security check needed here is the URL origin one. SW is
expected to be enabled always, not a permission granted per-process.

BUG=311631

Review URL: https://codereview.chromium.org/334413004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278326 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/service_worker/service_worker_dispatcher_host.cc

@@ -36,6 +36,22 @@ const uint32 kFilteredMessageClasses[] = {
   EmbeddedWorkerMsgStart,
 };
 
+bool CanRegisterServiceWorker(const GURL& document_url,
+                              const GURL& pattern,
+                              const GURL& script_url) {
+  // TODO: Respect Chrome's content settings, if we add a setting for
+  // controlling whether Service Worker is allowed.
+  return document_url.GetOrigin() == pattern.GetOrigin() &&
+         document_url.GetOrigin() == script_url.GetOrigin();
+}
+
+bool CanUnregisterServiceWorker(const GURL& document_url,
+                                const GURL& pattern) {
+  // TODO: Respect Chrome's content settings, if we add a setting for
+  // controlling whether Service Worker is allowed.
+  return document_url.GetOrigin() == pattern.GetOrigin();
+}
+
 }  // namespace
 
 ServiceWorkerDispatcherHost::ServiceWorkerDispatcherHost(
@@ -161,18 +177,6 @@ void ServiceWorkerDispatcherHost::OnRegisterServiceWorker(
     return;
   }
 
-  // TODO(alecflett): This check is insufficient for release. Add a
-  // ServiceWorker-specific policy query in
-  // ChildProcessSecurityImpl. See http://crbug.com/311631.
-  if (pattern.GetOrigin() != script_url.GetOrigin()) {
-    Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError(
-        thread_id,
-        request_id,
-        WebServiceWorkerError::ErrorTypeSecurity,
-        base::ASCIIToUTF16(kDomainMismatchErrorMessage)));
-    return;
-  }
-
   ServiceWorkerProviderHost* provider_host = GetContext()->GetProviderHost(
       render_process_id_, provider_id);
   if (!provider_host) {
@@ -188,6 +192,15 @@ void ServiceWorkerDispatcherHost::OnRegisterServiceWorker(
     return;
   }
 
+  if (!CanRegisterServiceWorker(
+      provider_host->document_url(), pattern, script_url)) {
+    Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError(
+        thread_id,
+        request_id,
+        WebServiceWorkerError::ErrorTypeSecurity,
+        base::ASCIIToUTF16(kDomainMismatchErrorMessage)));
+    return;
+  }
   GetContext()->RegisterServiceWorker(
       pattern,
       script_url,
@@ -204,9 +217,6 @@ void ServiceWorkerDispatcherHost::OnUnregisterServiceWorker(
     int request_id,
     int provider_id,
     const GURL& pattern) {
-  // TODO(alecflett): This check is insufficient for release. Add a
-  // ServiceWorker-specific policy query in
-  // ChildProcessSecurityImpl. See http://crbug.com/311631.
   if (!GetContext() || !ServiceWorkerUtils::IsFeatureEnabled()) {
     Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError(
         thread_id,
@@ -231,6 +241,15 @@ void ServiceWorkerDispatcherHost::OnUnregisterServiceWorker(
     return;
   }
 
+  if (!CanUnregisterServiceWorker(provider_host->document_url(), pattern)) {
+    Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError(
+        thread_id,
+        request_id,
+        WebServiceWorkerError::ErrorTypeSecurity,
+        base::ASCIIToUTF16(kDomainMismatchErrorMessage)));
+    return;
+  }
+
   GetContext()->UnregisterServiceWorker(
       pattern,
       base::Bind(&ServiceWorkerDispatcherHost::UnregistrationComplete,

content/browser/service_worker/service_worker_dispatcher_host_unittest.cc

@@ -23,29 +23,6 @@ namespace content {
 
 static const int kRenderProcessId = 1;
 
-class ServiceWorkerDispatcherHostTest : public testing::Test {
- protected:
-  ServiceWorkerDispatcherHostTest()
-      : browser_thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP) {}
-
-  virtual void SetUp() {
-    helper_.reset(new EmbeddedWorkerTestHelper(kRenderProcessId));
-  }
-
-  virtual void TearDown() {
-    helper_.reset();
-  }
-
-  ServiceWorkerContextCore* context() { return helper_->context(); }
-  ServiceWorkerContextWrapper* context_wrapper() {
-    return helper_->context_wrapper();
-  }
-
-  TestBrowserThreadBundle browser_thread_bundle_;
-  scoped_ptr<EmbeddedWorkerTestHelper> helper_;
-};
-
-
 class TestingServiceWorkerDispatcherHost : public ServiceWorkerDispatcherHost {
  public:
   TestingServiceWorkerDispatcherHost(
@@ -75,23 +52,113 @@ class TestingServiceWorkerDispatcherHost : public ServiceWorkerDispatcherHost {
   virtual ~TestingServiceWorkerDispatcherHost() {}
 };
 
+class ServiceWorkerDispatcherHostTest : public testing::Test {
+ protected:
+  ServiceWorkerDispatcherHostTest()
+      : browser_thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP) {}
+
+  virtual void SetUp() {
+    helper_.reset(new EmbeddedWorkerTestHelper(kRenderProcessId));
+    dispatcher_host_ = new TestingServiceWorkerDispatcherHost(
+        kRenderProcessId, context_wrapper(), helper_.get());
+  }
+
+  virtual void TearDown() {
+    helper_.reset();
+  }
+
+  ServiceWorkerContextCore* context() { return helper_->context(); }
+  ServiceWorkerContextWrapper* context_wrapper() {
+    return helper_->context_wrapper();
+  }
+
+  void Register(int64 provider_id,
+                GURL pattern,
+                GURL worker_url,
+                uint32 expected_message) {
+    dispatcher_host_->OnMessageReceived(
+        ServiceWorkerHostMsg_RegisterServiceWorker(
+            -1, -1, provider_id, pattern, worker_url));
+    base::RunLoop().RunUntilIdle();
+    EXPECT_TRUE(dispatcher_host_->ipc_sink()->GetUniqueMessageMatching(
+        expected_message));
+    dispatcher_host_->ipc_sink()->ClearMessages();
+  }
+
+  void Unregister(int64 provider_id, GURL pattern, uint32 expected_message) {
+    dispatcher_host_->OnMessageReceived(
+        ServiceWorkerHostMsg_UnregisterServiceWorker(
+            -1, -1, provider_id, pattern));
+    base::RunLoop().RunUntilIdle();
+    EXPECT_TRUE(dispatcher_host_->ipc_sink()->GetUniqueMessageMatching(
+        expected_message));
+    dispatcher_host_->ipc_sink()->ClearMessages();
+  }
+
+  TestBrowserThreadBundle browser_thread_bundle_;
+  scoped_ptr<EmbeddedWorkerTestHelper> helper_;
+  scoped_refptr<TestingServiceWorkerDispatcherHost> dispatcher_host_;
+};
+
 TEST_F(ServiceWorkerDispatcherHostTest, DisabledCausesError) {
   DCHECK(!CommandLine::ForCurrentProcess()->HasSwitch(
               switches::kEnableServiceWorker));
 
-  scoped_refptr<TestingServiceWorkerDispatcherHost> dispatcher_host =
-      new TestingServiceWorkerDispatcherHost(
-          kRenderProcessId, context_wrapper(), helper_.get());
-
-  dispatcher_host->OnMessageReceived(
+  dispatcher_host_->OnMessageReceived(
       ServiceWorkerHostMsg_RegisterServiceWorker(-1, -1, -1, GURL(), GURL()));
 
   // TODO(alecflett): Pump the message loop when this becomes async.
-  ASSERT_EQ(1UL, dispatcher_host->ipc_sink()->message_count());
-  EXPECT_TRUE(dispatcher_host->ipc_sink()->GetUniqueMessageMatching(
+  ASSERT_EQ(1UL, dispatcher_host_->ipc_sink()->message_count());
+  EXPECT_TRUE(dispatcher_host_->ipc_sink()->GetUniqueMessageMatching(
       ServiceWorkerMsg_ServiceWorkerRegistrationError::ID));
 }
 
+// TODO(falken): Enable this test when we remove the
+// --enable-service-worker-flag (see crbug.com/352581)
+TEST_F(ServiceWorkerDispatcherHostTest, DISABLED_RegisterSameOrigin) {
+  const int64 kProviderId = 99;  // Dummy value
+  scoped_ptr<ServiceWorkerProviderHost> host(new ServiceWorkerProviderHost(
+      kRenderProcessId, kProviderId, context()->AsWeakPtr(), NULL));
+  host->SetDocumentUrl(GURL("http://www.example.com/foo"));
+  base::WeakPtr<ServiceWorkerProviderHost> provider_host = host->AsWeakPtr();
+  context()->AddProviderHost(host.Pass());
+
+  Register(kProviderId,
+           GURL("http://www.example.com/*"),
+           GURL("http://foo.example.com/bar"),
+           ServiceWorkerMsg_ServiceWorkerRegistrationError::ID);
+  Register(kProviderId,
+           GURL("http://foo.example.com/*"),
+           GURL("http://www.example.com/bar"),
+           ServiceWorkerMsg_ServiceWorkerRegistrationError::ID);
+  Register(kProviderId,
+           GURL("http://foo.example.com/*"),
+           GURL("http://foo.example.com/bar"),
+           ServiceWorkerMsg_ServiceWorkerRegistrationError::ID);
+  Register(kProviderId,
+           GURL("http://www.example.com/*"),
+           GURL("http://www.example.com/bar"),
+           ServiceWorkerMsg_ServiceWorkerRegistered::ID);
+}
+
+// TODO(falken): Enable this test when we remove the
+// --enable-service-worker-flag (see crbug.com/352581)
+TEST_F(ServiceWorkerDispatcherHostTest, DISABLED_UnregisterSameOrigin) {
+  const int64 kProviderId = 99;  // Dummy value
+  scoped_ptr<ServiceWorkerProviderHost> host(new ServiceWorkerProviderHost(
+      kRenderProcessId, kProviderId, context()->AsWeakPtr(), NULL));
+  host->SetDocumentUrl(GURL("http://www.example.com/foo"));
+  base::WeakPtr<ServiceWorkerProviderHost> provider_host = host->AsWeakPtr();
+  context()->AddProviderHost(host.Pass());
+
+  Unregister(kProviderId,
+             GURL("http://foo.example.com/*"),
+             ServiceWorkerMsg_ServiceWorkerRegistrationError::ID);
+  Unregister(kProviderId,
+             GURL("http://www.example.com/*"),
+             ServiceWorkerMsg_ServiceWorkerUnregistered::ID);
+}
+
 // Disable this since now we cache command-line switch in
 // ServiceWorkerUtils::IsFeatureEnabled() and this could be flaky depending
 // on testing order. (crbug.com/352581)
@@ -103,19 +170,15 @@ TEST_F(ServiceWorkerDispatcherHostTest, DISABLED_Enabled) {
   CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kEnableServiceWorker);
 
-  scoped_refptr<TestingServiceWorkerDispatcherHost> dispatcher_host =
-      new TestingServiceWorkerDispatcherHost(
-          kRenderProcessId, context_wrapper(), helper_.get());
-
-  dispatcher_host->OnMessageReceived(
+  dispatcher_host_->OnMessageReceived(
       ServiceWorkerHostMsg_RegisterServiceWorker(-1, -1, -1, GURL(), GURL()));
   base::RunLoop().RunUntilIdle();
 
   // TODO(alecflett): Pump the message loop when this becomes async.
-  ASSERT_EQ(2UL, dispatcher_host->ipc_sink()->message_count());
-  EXPECT_TRUE(dispatcher_host->ipc_sink()->GetUniqueMessageMatching(
+  ASSERT_EQ(2UL, dispatcher_host_->ipc_sink()->message_count());
+  EXPECT_TRUE(dispatcher_host_->ipc_sink()->GetUniqueMessageMatching(
       EmbeddedWorkerMsg_StartWorker::ID));
-  EXPECT_TRUE(dispatcher_host->ipc_sink()->GetUniqueMessageMatching(
+  EXPECT_TRUE(dispatcher_host_->ipc_sink()->GetUniqueMessageMatching(
       ServiceWorkerMsg_ServiceWorkerRegistered::ID));
 }
 
@@ -125,56 +188,48 @@ TEST_F(ServiceWorkerDispatcherHostTest, EarlyContextDeletion) {
   CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kEnableServiceWorker);
 
-  scoped_refptr<TestingServiceWorkerDispatcherHost> dispatcher_host =
-      new TestingServiceWorkerDispatcherHost(
-          kRenderProcessId, context_wrapper(), helper_.get());
-
   helper_->ShutdownContext();
 
   // Let the shutdown reach the simulated IO thread.
   base::RunLoop().RunUntilIdle();
 
-  dispatcher_host->OnMessageReceived(
+  dispatcher_host_->OnMessageReceived(
       ServiceWorkerHostMsg_RegisterServiceWorker(-1, -1, -1, GURL(), GURL()));
 
   // TODO(alecflett): Pump the message loop when this becomes async.
-  ASSERT_EQ(1UL, dispatcher_host->ipc_sink()->message_count());
-  EXPECT_TRUE(dispatcher_host->ipc_sink()->GetUniqueMessageMatching(
+  ASSERT_EQ(1UL, dispatcher_host_->ipc_sink()->message_count());
+  EXPECT_TRUE(dispatcher_host_->ipc_sink()->GetUniqueMessageMatching(
       ServiceWorkerMsg_ServiceWorkerRegistrationError::ID));
 }
 
 TEST_F(ServiceWorkerDispatcherHostTest, ProviderCreatedAndDestroyed) {
-  scoped_refptr<TestingServiceWorkerDispatcherHost> dispatcher_host =
-      new TestingServiceWorkerDispatcherHost(
-          kRenderProcessId, context_wrapper(), helper_.get());
-
   const int kProviderId = 1001;  // Test with a value != kRenderProcessId.
 
-  dispatcher_host->OnMessageReceived(
+  dispatcher_host_->OnMessageReceived(
       ServiceWorkerHostMsg_ProviderCreated(kProviderId));
   EXPECT_TRUE(context()->GetProviderHost(kRenderProcessId, kProviderId));
 
   // Two with the same ID should be seen as a bad message.
-  dispatcher_host->OnMessageReceived(
+  dispatcher_host_->OnMessageReceived(
       ServiceWorkerHostMsg_ProviderCreated(kProviderId));
-  EXPECT_EQ(1, dispatcher_host->bad_messages_received_count_);
+  EXPECT_EQ(1, dispatcher_host_->bad_messages_received_count_);
 
-  dispatcher_host->OnMessageReceived(
+  dispatcher_host_->OnMessageReceived(
       ServiceWorkerHostMsg_ProviderDestroyed(kProviderId));
   EXPECT_FALSE(context()->GetProviderHost(kRenderProcessId, kProviderId));
 
   // Destroying an ID that does not exist warrants a bad message.
-  dispatcher_host->OnMessageReceived(
+  dispatcher_host_->OnMessageReceived(
       ServiceWorkerHostMsg_ProviderDestroyed(kProviderId));
-  EXPECT_EQ(2, dispatcher_host->bad_messages_received_count_);
+  EXPECT_EQ(2, dispatcher_host_->bad_messages_received_count_);
 
   // Deletion of the dispatcher_host should cause providers for that
   // process to get deleted as well.
-  dispatcher_host->OnMessageReceived(
+  dispatcher_host_->OnMessageReceived(
       ServiceWorkerHostMsg_ProviderCreated(kProviderId));
   EXPECT_TRUE(context()->GetProviderHost(kRenderProcessId, kProviderId));
-  EXPECT_TRUE(dispatcher_host->HasOneRef());
-  dispatcher_host = NULL;
+  EXPECT_TRUE(dispatcher_host_->HasOneRef());
+  dispatcher_host_ = NULL;
   EXPECT_FALSE(context()->GetProviderHost(kRenderProcessId, kProviderId));
 }