It seems that this has regressed in https://codereview.chromium.org/2002943002

I see that estark@ commented in the CR saying: https://codereview.chromium.org/2002943002/#msg5:
[...] can you add something to the CL description like, "Specifically, as of
this patch, non-redirected blocked URLs are reported with the full URL, even if
it is cross-origin"?

Does the above mean that reporting the full URL is okay cross-origin?  Disclosing URIs cross-site seems wrong.

Some printf-style debugging from running unmodifed http/tests/security/contentSecurityPolicy/frame-src-child-frame-navigates-to-blocked-origin.html layout test (where I would expect blocked uri to be stripped to origin-only):

This is the printf-style debugging I've added:

static String stripURLForUseInReport(Document* document, const KURL& url, RedirectStatus redirectStatus)
{
    if (!url.isValid())
        return String();
    if (!url.isHierarchical() || url.protocolIs("file"))
        return url.protocol();
    if (redirectStatus == RedirectStatus::NoRedirect || document->getSecurityOrigin()->canRequest(url)) {
        std::cerr << "stripURLForUseInReport"
                  << "; redirectStatus=" << static_cast<int>(redirectStatus)
                  << "; canRequest=" << document->getSecurityOrigin()->canRequest(url)
                  << "; doc->secOrigin=" << document->getSecurityOrigin()->toString().utf8().data()
                  << "; url=" << url.getString().utf8().data()
                  << "; stack=" << base::debug::StackTrace().ToString()
                  << std::endl;

And the result is:

stripURLForUseInReport; redirectStatus=1; canRequest=0; doc->secOrigin=http://127.0.0.1:8000; url=http://localhost:8000/security/contentSecurityPolicy/resources/target-for-frame-that-navigates-itself.html; stack=#0 0x0000004c0571 __interceptor_backtrace
#1 0x7f5a88ecd6f3 base::debug::StackTrace::StackTrace()
#2 0x7f5a7a5717ed blink::stripURLForUseInReport()
#3 0x7f5a7a5638c3 blink::ContentSecurityPolicy::reportViolation()
#4 0x7f5a7a534fb7 blink::CSPDirectiveList::reportViolation()
#5 0x7f5a7a53d49e blink::CSPDirectiveList::checkSourceAndReportViolation()
#6 0x7f5a7a5437af blink::CSPDirectiveList::allowChildFrameFromSource()
#7 0x7f5a7a5613d5 blink::isAllowedByAllWithURL<>()
#8 0x7f5a7a7e9f38 blink::FrameLoader::shouldContinueForNavigationPolicy()
#9 0x7f5a7a7e5947 blink::FrameLoader::startLoad()
#10 0x7f5a7a7da9c0 blink::FrameLoader::load()
#11 0x7f5a79607811 blink::HTMLAnchorElement::handleClick()

In particular I note that the origin of the protected document (http://127.0.0.1:8000) is different from the origin of the blocked uri (http://localhost:8000).  So - according to spec (and to block cross-origin disclosure of the URIs) we should strip the blocked URI to just its origin, right?

CSP3 changes the way reporting works; we ought to be reporting the originally requested URL in all cases. We can't quite do that yet, which is why we've ended up with the redirect logic you've noted above.

The thought behind that is that the page knows what resources it's loaded. That data is exposed in a number of ways, ranging from scraping the DOM during the `securitypolicyviolation` event on `document` to examining timing APIs, etc. Reporting to itself what resources it intentionally loaded doesn't seem like a violation of SOP.

Navigations initiated by the `<iframe>` contents are an interesting corner case. I think you're correct that we should probably be reporting the `src` of the containing frame element in those cases, rather than the endpoint to which the user navigated.

Would you like to take on that fix, lukasza@, or shall I? I'll adjust the spec in any event.

Issue 633348 has been merged into this issue.

RE: Would you like to take on that fix, lukasza@, or shall I? I'll adjust the spec in any event.

Feel free to take the bug's ownership.  I put myself as the owner, because CSP2 compliance (i.e. https://www.w3.org/TR/CSP2/#strip-uri-for-reporting) seemed simple to fix, but I am not sure what is the right path going forward with CSP3.


RE: we should probably be reporting the `src` of the containing frame element [...] rather than the endpoint to which the user navigated

Maybe :-)

What about srcdoc / about:blank frames?  Would the get empty blockedURI (or blockedURI would be stripped to just the scheme)?

What is the thinking behind CSP3 changes?  Are the changes meant to make it easier to identify what causes CSP violations?  Maybe we can leave the simple CSP2 rules for blockedURI alone, and add an extra property to the report (originalUriRequest?).


RE: Navigations initiated by the `<iframe>` contents

You are right that navigations initiated by the protected document are not interesting - report violations do not disclose anything new to the protected document in these cases.

Can we reliably recognize such navigations?  Note that these don't necessarily have to be navigations initiated by child frame's javascript, but can also by initiated by user interactions with the child frame.

Assigning to mkwst@ as per #6. Please re-assign as needed.

M53 Stable launch is coming soon.Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix asap so it gets chance to bake in beta before stable promotion. Thank you.

mkwst@'s calendar suggests he's on leave - anybody else who could take a look, eisinger@?

M53 Stable launch is coming VERY soon.Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix asap so it gets chance to bake in beta before stable promotion later this month. Thank you.

eisinger: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94a6ff53682eac87184c1682b63faf6110325174

commit 94a6ff53682eac87184c1682b63faf6110325174
Author: mkwst <mkwst@chromium.org>
Date: Thu Aug 18 13:18:16 2016

CSP: Strip reported URLs for 'frame-src' and 'object-src'.

The relaxation that landed in https://codereview.chromium.org/2002943002
was a bit too relaxed, and leaks navigation targets cross-origin for
'frame-src' and 'object-src' violations.

This patch reverts to the old behavior for those two directives.

BUG= 633306 

Review-Url: https://codereview.chromium.org/2255103002
Cr-Commit-Position: refs/heads/master@{#412809}

[delete] https://crrev.com/21f7b3ae303e08aaccd054870bf5b3978589d702/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-blocked-expected.txt
[modify] https://crrev.com/94a6ff53682eac87184c1682b63faf6110325174/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-blocked.html
[delete] https://crrev.com/21f7b3ae303e08aaccd054870bf5b3978589d702/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-child-frame-navigates-to-blocked-origin-expected.txt
[modify] https://crrev.com/94a6ff53682eac87184c1682b63faf6110325174/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-child-frame-navigates-to-blocked-origin.html
[delete] https://crrev.com/21f7b3ae303e08aaccd054870bf5b3978589d702/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-redirect-blocked-expected.txt
[modify] https://crrev.com/94a6ff53682eac87184c1682b63faf6110325174/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-redirect-blocked.html
[modify] https://crrev.com/94a6ff53682eac87184c1682b63faf6110325174/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/target-for-frame-that-navigates-itself.html
[modify] https://crrev.com/94a6ff53682eac87184c1682b63faf6110325174/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp

mkwst@, please request a merge to M53 as soon as change is baked/verified in canary as we're VERY close to M53 stable promotion and this bug is M53 Stable Blocker. Thank you.

Requesting a merge; the change is small and should be safe. I'll verify in today's Canary before doing anything crazy, though. :)

Your change meets the bar and is auto-approved for M53 (branch: 2785)

Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Please try to merge your change to M53 branch 2785 today before 5:00 PM PT or latest by 4:00 PM PT on Monday so we can pick it up for next week LAST M53 beta release.  Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/27294e2c97721a069b74202d735e89b1facac756

commit 27294e2c97721a069b74202d735e89b1facac756
Author: Mike West <mkwst@google.com>
Date: Mon Aug 22 07:46:26 2016

CSP: Strip reported URLs for 'frame-src' and 'object-src'.

The relaxation that landed in https://codereview.chromium.org/2002943002
was a bit too relaxed, and leaks navigation targets cross-origin for
'frame-src' and 'object-src' violations.

This patch reverts to the old behavior for those two directives.

BUG= 633306 

Review-Url: https://codereview.chromium.org/2255103002
Cr-Commit-Position: refs/heads/master@{#412809}
(cherry picked from commit 94a6ff53682eac87184c1682b63faf6110325174)

Review URL: https://codereview.chromium.org/2267653003 .

Cr-Commit-Position: refs/branch-heads/2785@{#702}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[delete] https://crrev.com/566ac45e693fb76a0dd6ff84f489e5287bf2c055/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-blocked-expected.txt
[modify] https://crrev.com/27294e2c97721a069b74202d735e89b1facac756/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-blocked.html
[delete] https://crrev.com/566ac45e693fb76a0dd6ff84f489e5287bf2c055/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-child-frame-navigates-to-blocked-origin-expected.txt
[modify] https://crrev.com/27294e2c97721a069b74202d735e89b1facac756/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-child-frame-navigates-to-blocked-origin.html
[delete] https://crrev.com/566ac45e693fb76a0dd6ff84f489e5287bf2c055/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-redirect-blocked-expected.txt
[modify] https://crrev.com/27294e2c97721a069b74202d735e89b1facac756/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-redirect-blocked.html
[modify] https://crrev.com/27294e2c97721a069b74202d735e89b1facac756/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/target-for-frame-that-navigates-itself.html
[modify] https://crrev.com/27294e2c97721a069b74202d735e89b1facac756/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot