service worker: Don't control a subframe of an insecure context

content/child/service_worker/service_worker_network_provider.cc

 // Copyright 2014 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/child/service_worker/service_worker_network_provider.h"
 
 #include "base/atomic_sequence_num.h"
 #include "content/child/child_thread_impl.h"
 #include "content/child/service_worker/service_worker_provider_context.h"
 #include "content/common/navigation_params.h"
 #include "content/common/service_worker/service_worker_messages.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/common/browser_side_navigation_policy.h"
+#include "third_party/WebKit/public/platform/WebSecurityOrigin.h"
+#include "third_party/WebKit/public/platform/WebString.h"
+#include "third_party/WebKit/public/web/WebLocalFrame.h"
+#include "third_party/WebKit/public/web/WebSandboxFlags.h"
 
 namespace content {
 
 namespace {
 
 const char kUserDataKey[] = "SWProviderKey";
 
 // Must be unique in the child process.
 int GetNextProviderId() {
   static base::StaticAtomicSequenceNumber sequence;
@@ skipping 21 matching lines @@
     base::SupportsUserData* datasource_userdata) {
   return static_cast<ServiceWorkerNetworkProvider*>(
       datasource_userdata->GetUserData(&kUserDataKey));
 }
 
 // static
 std::unique_ptr<ServiceWorkerNetworkProvider>
 ServiceWorkerNetworkProvider::CreateForNavigation(
     int route_id,
     const RequestNavigationParams& request_params,
-    blink::WebSandboxFlags sandbox_flags,
+    blink::WebLocalFrame* frame,
     bool content_initiated) {
   bool browser_side_navigation = IsBrowserSideNavigationEnabled();
   bool should_create_provider_for_window = false;
   int service_worker_provider_id = kInvalidServiceWorkerProviderId;
   std::unique_ptr<ServiceWorkerNetworkProvider> network_provider;
 
   // Determine if a ServiceWorkerNetworkProvider should be created and properly
   // initialized for the navigation. A default ServiceWorkerNetworkProvider
   // will always be created since it is expected in a certain number of places,
   // however it will have an invalid id.
   // PlzNavigate: |service_worker_provider_id| can be sent by the browser, if
   // it already created the SeviceWorkerProviderHost.
   if (browser_side_navigation && !content_initiated) {
     should_create_provider_for_window =
         request_params.should_create_service_worker;
     service_worker_provider_id = request_params.service_worker_provider_id;
     DCHECK(ServiceWorkerUtils::IsBrowserAssignedProviderId(
                service_worker_provider_id) ||
            service_worker_provider_id == kInvalidServiceWorkerProviderId);
   } else {
     should_create_provider_for_window =
-        (sandbox_flags & blink::WebSandboxFlags::Origin) !=
+        (frame->effectiveSandboxFlags() & blink::WebSandboxFlags::Origin) !=
         blink::WebSandboxFlags::Origin;
+    // Check if |frame| is a subframe of an insecure context.
+    // |frame|'s document is not yet created, so start with the parent.
+    blink::WebFrame* parent = frame->parent();
+    while (parent && should_create_provider_for_window) {

[falken, 2016/05/24 13:16:37]

This ancestor walk should probably be some utility function like
Document::isSecureContext(). One subtle difference is we shouldn't check for
scheme exceptions like Document does, since those shouldn't be applied to child
frames, which is what we care about here.

[Marijn Kruisselbrink, 2016/05/24 17:03:35]

Also keep in mind that to truly match what is specified in
https://w3c.github.io/webappsec-secure-contexts/#settings-object you need to not
only exclude iframes inside insecure contexts, but also top-level windows opened
by insecure contexts without no-opener.

[jww, 2016/05/24 18:06:03]

I have a strong preference to factor this out into a shared utility function
that Document and this both use. It will be much easier to track changes, and
even to just track that this check exists at all. This would also probably give
you a helpful error message to use, no?

Also, why don't you check if the current origin is potentially trustworthy? That
is, you only start checking the child frames. I think this is related to mek's
comment.

[falken, 2016/05/25 01:33:34]

I can't start with the current frame because document is not yet created.
getSecurityOrigin() calls frame()->securityContext()->getSecurityOrigin(), but
securityContext() is just document(), so it blows up.

I probably do need to check the current frame to address mek's point. Ideas?

[falken, 2016/05/25 04:09:45]

Does Chrome currently check for insecure opener when deciding whether a context
is secure? It looks like Document::isSecureContext() doesn't check it. If I do
window.open from an http page to an https page, I can register a service worker
from the opened page.

[jww, 2016/05/28 01:35:24]

That's a great question. I believe the answer is "no", we don't do that yet.

+      blink::WebSecurityOrigin securityOrigin = parent->getSecurityOrigin();
+      if (!securityOrigin.isPotentiallyTrustworthy())
+        should_create_provider_for_window = false;
+      parent = parent->parent();
+    }
   }
 
   // Now create the ServiceWorkerNetworkProvider (with invalid id if needed).
   if (should_create_provider_for_window) {
     if (service_worker_provider_id == kInvalidServiceWorkerProviderId) {
       network_provider = std::unique_ptr<ServiceWorkerNetworkProvider>(
           new ServiceWorkerNetworkProvider(route_id,
                                            SERVICE_WORKER_PROVIDER_FOR_WINDOW));
     } else {
       CHECK(browser_side_navigation);
@@ skipping 53 matching lines @@
     return;  // May be null in some tests.
   ChildThreadImpl::current()->Send(
       new ServiceWorkerHostMsg_SetVersionId(provider_id_, version_id));
 }
 
 bool ServiceWorkerNetworkProvider::IsControlledByServiceWorker() const {
   return context() && context()->controller();
 }
 
 }  // namespace content