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

content/child/service_worker/service_worker_network_provider.cc

Index: content/child/service_worker/service_worker_network_provider.cc
diff --git a/content/child/service_worker/service_worker_network_provider.cc b/content/child/service_worker/service_worker_network_provider.cc
index dab3db7517595f4598357183fb9f3af54ad28a42..e22da3534f9207c1bfd534ad8431a6b2e55a64cf 100644
--- a/content/child/service_worker/service_worker_network_provider.cc
+++ b/content/child/service_worker/service_worker_network_provider.cc
@@ -11,6 +11,10 @@
 #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 {
 
@@ -52,7 +56,7 @@ 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;
@@ -74,8 +78,17 @@ ServiceWorkerNetworkProvider::CreateForNavigation(
            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).