Allow SW registration only if it's secure AND it's HTTP or HTTPS

content/browser/service_worker/service_worker_dispatcher_host.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_dispatcher_host.h"
 
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
 #include "content/browser/message_port_message_filter.h"
@@ skipping 16 matching lines @@
 #include "url/gurl.h"
 
 using blink::WebServiceWorkerError;
 
 namespace content {
 
 namespace {
 
 const char kNoDocumentURLErrorMessage[] =
     "No URL is associated with the caller's document.";
+const char kDisallowedURLErrorMessage[] =
+    "The URL is not supported.";

[nhiroki, 2015/02/02 08:37:43]

Ideally we should show more descriptive message specialized for each case as
falken@ has recently worked (https://codereview.chromium.org/845313004/), but I
think it's ok to keep as is in order to merge easier.

 const char kShutdownErrorMessage[] =
     "The Service Worker system has shutdown.";
 const char kUserDeniedPermissionMessage[] =
     "The user denied permission to use Service Worker.";
 
 const uint32 kFilteredMessageClasses[] = {
   ServiceWorkerMsgStart,
   EmbeddedWorkerMsgStart,
 };
 
 bool AllOriginsMatch(const GURL& url_a, const GURL& url_b, const GURL& url_c) {
   return url_a.GetOrigin() == url_b.GetOrigin() &&
          url_a.GetOrigin() == url_c.GetOrigin();
 }
 
 // TODO(dominicc): When crbug.com/362214 is fixed use that to be
 // consistent with Blink's
 // SecurityOrigin::canAccessFeatureRequiringSecureOrigin.
 bool OriginCanAccessServiceWorkers(const GURL& url) {
-  return url.SchemeIsSecure() || net::IsLocalhost(url.host());
+  return url.SchemeIsHTTPOrHTTPS() &&
+      (url.SchemeIsSecure() || net::IsLocalhost(url.host()));
 }
 
 bool CanRegisterServiceWorker(const GURL& document_url,
                               const GURL& pattern,
                               const GURL& script_url) {
   DCHECK(document_url.is_valid());
   DCHECK(pattern.is_valid());
   DCHECK(script_url.is_valid());
   return AllOriginsMatch(document_url, pattern, script_url) &&
-         OriginCanAccessServiceWorkers(document_url);
+         OriginCanAccessServiceWorkers(document_url) &&
+         OriginCanAccessServiceWorkers(pattern) &&

[falken, 2015/02/02 08:44:04]

Actually, doesn't AllOriginsMatch mean you only need to call
OriginCanAccessServiceWorkers on one of the args?

+         OriginCanAccessServiceWorkers(script_url);
 }
 
 bool CanUnregisterServiceWorker(const GURL& document_url,
                                 const GURL& pattern) {
   DCHECK(document_url.is_valid());
   DCHECK(pattern.is_valid());
   return document_url.GetOrigin() == pattern.GetOrigin() &&
-         OriginCanAccessServiceWorkers(document_url);
+         OriginCanAccessServiceWorkers(document_url) &&
+         OriginCanAccessServiceWorkers(pattern);
 }
 
 bool CanGetRegistration(const GURL& document_url,
                         const GURL& given_document_url) {
   DCHECK(document_url.is_valid());
   DCHECK(given_document_url.is_valid());
   return document_url.GetOrigin() == given_document_url.GetOrigin() &&
          OriginCanAccessServiceWorkers(document_url);

[nhiroki, 2015/02/02 08:37:43]

Can we also check |given_document_url| so that we can reject an invalid URL sent
from a compromised renderer?

  "return ... && OriginCanAccessServiceWorkers(given_document_url);"

[kinuko, 2015/02/02 09:43:24]

Done.

 }
 
 }  // namespace
 
 ServiceWorkerDispatcherHost::ServiceWorkerDispatcherHost(
     int render_process_id,
     MessagePortMessageFilter* message_port_message_filter,
     ResourceContext* resource_context)
     : BrowserMessageFilter(kFilteredMessageClasses,
                            arraysize(kFilteredMessageClasses)),
@@ skipping 194 matching lines @@
   if (provider_host->document_url().is_empty()) {
     Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError(
         thread_id, request_id, WebServiceWorkerError::ErrorTypeSecurity,
         base::ASCIIToUTF16(kServiceWorkerRegisterErrorPrefix) +
             base::ASCIIToUTF16(kNoDocumentURLErrorMessage)));
     return;
   }
 
   if (!CanRegisterServiceWorker(
       provider_host->document_url(), pattern, script_url)) {
-    BadMessageReceived();
+    // TODO(kinuko): Change this back to BadMessageReceived() once we start
+    // to check these in the renderer too. (http://crbug.com/453982)
+    Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError(
+        thread_id, request_id, WebServiceWorkerError::ErrorTypeSecurity,
+        base::ASCIIToUTF16(kServiceWorkerRegisterErrorPrefix) +
+            base::ASCIIToUTF16(kDisallowedURLErrorMessage)));
     return;
   }
 
   std::string error_message;
   if (ServiceWorkerUtils::ContainsDisallowedCharacter(pattern, script_url,
                                                       &error_message)) {
     Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError(
         thread_id, request_id, WebServiceWorkerError::ErrorTypeSecurity,
         base::ASCIIToUTF16(kServiceWorkerRegisterErrorPrefix) +
             base::UTF8ToUTF16(error_message)));
@@ skipping 64 matching lines @@
   if (provider_host->document_url().is_empty()) {
     Send(new ServiceWorkerMsg_ServiceWorkerUnregistrationError(
         thread_id,
         request_id,
         WebServiceWorkerError::ErrorTypeSecurity,
         base::ASCIIToUTF16(kNoDocumentURLErrorMessage)));
     return;
   }
 
   if (!CanUnregisterServiceWorker(provider_host->document_url(), pattern)) {
-    BadMessageReceived();
+    // TODO(kinuko): Change this back to BadMessageReceived() once we start
+    // to check these in the renderer too. (http://crbug.com/453982)
+    Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError(
+        thread_id, request_id, WebServiceWorkerError::ErrorTypeSecurity,
+        base::ASCIIToUTF16(kServiceWorkerRegisterErrorPrefix) +

[falken, 2015/02/02 08:44:04]

kServiceWorkerUnregisterErrorPrefix

[kinuko, 2015/02/02 09:43:24]

Done.

Also fixed the error msg type (RegistrationError -> UnregistrationError)

+            base::ASCIIToUTF16(kDisallowedURLErrorMessage)));
     return;
   }
 
   if (!GetContentClient()->browser()->AllowServiceWorker(
           pattern, provider_host->topmost_frame_url(), resource_context_)) {
     Send(new ServiceWorkerMsg_ServiceWorkerUnregistrationError(
         thread_id,
         request_id,
         WebServiceWorkerError::ErrorTypeUnknown,
         base::ASCIIToUTF16(kUserDeniedPermissionMessage)));
@@ skipping 545 matching lines @@
   ServiceWorkerHandle* handle = handles_.Lookup(handle_id);
   if (!handle) {
     BadMessageReceived();
     return;
   }
   handle->version()->StopWorker(
       base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
 }
 
 }  // namespace content