Add scheme exceptions for isSecureContext

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) Research In Motion Limited 2010-2011. All rights reserved.
@@ skipping 5646 matching lines @@
 bool Document::isSecureContext(String& errorMessage, const SecureContextCheck privilegeContextCheck) const
 {
     if (SecurityContext::isSandboxed(SandboxOrigin)) {
         if (!SecurityOrigin::create(url())->isPotentiallyTrustworthy(errorMessage))
             return false;
     } else {
         if (!securityOrigin()->isPotentiallyTrustworthy(errorMessage))
             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) {
             // Skip to the next ancestor if it's a srcdoc.
             if (!context->isSrcdocDocument()) {
                 if (context->securityContext().isSandboxed(SandboxOrigin)) {
                     // For a sandboxed origin, use the document's URL.
                     RefPtr<SecurityOrigin> origin = SecurityOrigin::create(context->url());
                     if (!origin->isPotentiallyTrustworthy(errorMessage))
                         return false;
@@ skipping 80 matching lines @@
 #ifndef NDEBUG
 using namespace blink;
 void showLiveDocumentInstances()
 {
     Document::WeakDocumentSet& set = Document::liveDocumentSet();
     fprintf(stderr, "There are %u documents currently alive:\n", set.size());
     for (Document* document : set)
         fprintf(stderr, "- Document %p URL: %s\n", document, document->url().string().utf8().data());
 }
 #endif