Add scheme exceptions for isSecureContext

third_party/WebKit/Source/core/dom/Document.cpp

Index: third_party/WebKit/Source/core/dom/Document.cpp
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp
index f0573887629fc669ea47dfd44cdb3d1389c40f38..ea3606509e4dc01258270b1cab6cbd66b4756d31 100644
--- a/third_party/WebKit/Source/core/dom/Document.cpp
+++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -5664,6 +5664,9 @@ bool Document::isSecureContext(String& errorMessage, const SecureContextCheck pr
             return false;
     }
 
+    if (SecurityPolicy::shouldOriginBypassSecureContextCheck(*securityOrigin()))
+        return true;
+

[robwu, 2015/10/03 10:25:08]

Putting this check here implies that the origin also needs to pass
isPotentiallyTrustworthy (chrome-extension: does pass it). Was this requirement
intentional?

The check seems insufficient for <iframe src="page.html" sandbox="allow-scripts
allow-same-origin"></iframe>. Try this (which also solves the issue I mentioned
above):

    if (SecurityContext::isSandboxed(SandboxOrigin)) {
        RefPtr<SecurityOrigin> origin = SecurityOrigin::create(context->url());
        if (SecurityPolicy::shouldOriginBypassSecureContextCheck(*origin))
            return true;
        if (!origin->isPotentiallyTrustworthy(errorMessage))
            return false;
    } else {
        if
(SecurityPolicy::shouldOriginBypassSecureContextCheck(*securityOrigin()))
            return true;
        if (!securityOrigin()->isPotentiallyTrustworthy(errorMessage))
            return false;
    }

[jww, 2015/10/03 17:15:06]

Yes, this is intentional because an origin should be trustworthy before we're
willing to allow a bypass of the ancestor check.
Is your concern is a chrome-extensions: frame wanting to make a sandboxed
iframe? If that's the case, then I'd love to get Devlin's opinion on doing this,
since the general idea was to avoid children of chrome-extension: frames getting
an exception, but I agree that in this very specific case (a direct sandboxed
child) it does make sense to have an exception.

[robwu, 2015/10/03 17:27:09]

Yes, with the sandboxed frame being at the chrome-extension: scheme AND
allow-same-origin set. Access is still blocked at other schemes (including
about:srcdoc/blank) are still blocked.

From the web dev/extension author's perspective, <iframe> and <iframe
sandbox=allow-same-origin> should behave identically for origin-based decisions,
right?

[jww, 2015/10/03 17:56:59]

I *think* that makes sense, although I'm still going to leave the
isPotentiallyTrustworthy check first, since we should never allow anything
sensitive on an untrustworthy origin. I'll update the CL, but we'll need
confirmation from Devlin and Mike.

[robwu, 2015/10/03 19:28:50]

When I wrote my comment, I mistakenly assumed that the if-block is for
sandbox=allow-same-origin, and the else-block for everything else (including
sandbox with unique origin).
Now I see that the first one means "sandbox with unique origin", and the second
one is for everything else, my argument about developer's expectations is no
longer valid.

But the change (patchset 2) is still okay, because it follows the spirit of the
Powerful features spec by ignoring the allow-same-origin sandbox flag (5.1, step
1.2.

     if (privilegeContextCheck == StandardSecureContextCheck) {
         Document* context = parentDocument();
         while (context) {