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

third_party/WebKit/public/web/WebFrame.h

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 534 matching lines @@
     virtual WebFrameImplBase* toImplBase() = 0;
     // TODO(dcheng): Fix const-correctness issues and remove this overload.
     virtual const WebFrameImplBase* toImplBase() const
     {
         return const_cast<WebFrame*>(this)->toImplBase();
     }
 
     // Returns the frame inside a given frame or iframe element. Returns 0 if
     // the given element is not a frame, iframe or if the frame is empty.
     BLINK_EXPORT static WebFrame* fromFrameOwnerElement(const WebElement&);
+    BLINK_EXPORT bool canHaveSecureChild(WebString& errorMessage) const;

[alexmos, 2016/05/26 22:13:35]

Do we need both this and WebDocument::isSecureContext in the public API?  Seems
WebDocument::isSecureContext is only used in plugins, where it's called on the
plugin container frame.  Could that just use canHaveSecureChild?

Also, since both uses don't care about the error message, maybe we don't need it
in the public API either?

[falken, 2016/05/27 05:47:04]

I think there's two problems there:
1) isSecureContext allows for scheme exceptions and canHaveSecureChild doesn't.
2) A Document can be frameless (apparently. the comments for WebDocument::frame
say so). So document.isSecureContext() can't be transformed to
document.frame()->canHaveSecureChild().

1) could probably be resolved by refactoring to a common method, but 2) looks
more difficult.

To be more specific, IIRC, the plugin situation looks like:
- Frame
  - Document
    ...
      - plugin container (Element)
        - plugin instance

It's calling isSecureContext on the document. So it can be whitelisted by
scheme. If we instead call Frame::canHaveSecureChild, the scheme whitelist
doesn't happen. And we might not have a frame.

Probably canHaveSecureChild should be renamed to something like
canHaveSecureChildFrameIgnoringSchemeExceptions.

Having said all that, the plugin check is just for metrics and isn't a security
check. So maybe it's ok being slightly "wrong" there? +raymes
I can certainly remove it.

[alexmos, 2016/05/27 22:44:30]

Yeah, I'm curious if the plugin case cares about scheme exceptions at all. 
Looks like it was added (by raymes@ in r329895) before scheme exceptions were
introduced (by jww@ in r354650), so I'll let jww@ and raymes@ comment. :)

[raymes, 2016/05/30 01:08:08]

The plugin case is just for metrics - we were just testing how frequently
plugins were loaded in secure contexts (that is, it's src is https and it's
parents are https). If we get something which approximates that it should be
fine especially if it simplifies code.

jww: Do we even still need those metrics? timwillis may know.

The metrics are: Pepper.PluginContextSecurity.TCPConnect and
Pepper.PluginContextSecurity.UDPBind

 
 #if BLINK_IMPLEMENTATION
     static WebFrame* fromFrame(Frame*);
 
     bool inShadowTree() const { return m_scope == WebTreeScopeType::Shadow; }
 
     static void traceFrames(Visitor*, WebFrame*);
     static void traceFrames(InlinedGlobalMarkingVisitor, WebFrame*);
     void clearWeakFrames(Visitor*);
     void clearWeakFrames(InlinedGlobalMarkingVisitor);
@@ skipping 33 matching lines @@
     WebFrame* m_firstChild;
     WebFrame* m_lastChild;
 
     WebFrame* m_opener;
     WebPrivateOwnPtr<OpenedFrameTracker> m_openedFrameTracker;
 };
 
 } // namespace blink
 
 #endif