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 34 matching lines @@
 };
 
 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) {

[nhiroki, 2015/02/02 06:14:27]

This function is only used for document_url. Probably we should also check
scope_url and script_url.

[kinuko, 2015/02/02 07:59:42]

Done.

-  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);
@@ skipping 220 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();

[falken, 2015/02/02 06:01:23]

With the new restriction, it's probably over-aggressive to kill the renderer
now. The original idea was that Blink-side does the sanity checking first, so if
it fails in Chrome we know the renderer has been compromised. I think we should
change this to sending a RegistrationError.

[kinuko, 2015/02/02 07:59:42]

I looked into this deeper, actually we do check if protocol's http or https for
document url in the renderer too but we don't do so for script or pattern URL.
For a band-aid I've made fixes in the browser side only and changed these
RegistrationError (for easier merge).

     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();

[falken, 2015/02/02 06:01:23]

Same here

[kinuko, 2015/02/02 07:59:42]

Done.

     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 49 matching lines @@
   // TODO(ksakamoto): This check can be removed once crbug.com/439697 is fixed.
   if (provider_host->document_url().is_empty()) {
     Send(new ServiceWorkerMsg_ServiceWorkerGetRegistrationError(
         thread_id, request_id, WebServiceWorkerError::ErrorTypeSecurity,
         base::ASCIIToUTF16(kServiceWorkerGetRegistrationErrorPrefix) +
             base::ASCIIToUTF16(kNoDocumentURLErrorMessage)));
     return;
   }
 
   if (!CanGetRegistration(provider_host->document_url(), document_url)) {
     BadMessageReceived();

[falken, 2015/02/02 06:01:23]

Same here.

[kinuko, 2015/02/02 07:59:42]

I assume this should be fine, as we check the scheme for document URL in the
renderer.

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