Bug 58643 - CSP frame-src is missing
Summary: CSP frame-src is missing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks: 53572
  Show dependency treegraph
 
Reported: 2011-04-15 00:23 PDT by Adam Barth
Modified: 2011-04-22 16:18 PDT (History)
5 users (show)

See Also:


Attachments
Patch (10.12 KB, patch)
2011-04-20 12:03 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (9.86 KB, patch)
2011-04-20 14:34 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch for landing (9.15 KB, patch)
2011-04-20 19:38 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (8.28 KB, patch)
2011-04-22 10:59 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2011-04-15 00:23:56 PDT
CSP frame-src is missing
Comment 1 Adam Barth 2011-04-20 12:03:37 PDT
Created attachment 90377 [details]
Patch
Comment 2 Adam Barth 2011-04-20 12:04:04 PDT
EWS failures expected because this patch depends on Bug 58646.
Comment 3 Eric Seidel (no email) 2011-04-20 13:28:22 PDT
Comment on attachment 90377 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90377&action=review

LGTM.  You said you were gonna make the message reporting hotter before landing.

> Source/WebCore/html/HTMLFrameElementBase.cpp:81
> +    // the Content-Security-Policy of the parent frame or the requestor.

requester?
Comment 4 Adam Barth 2011-04-20 14:34:53 PDT
Created attachment 90417 [details]
Patch
Comment 5 Adam Barth 2011-04-20 19:38:36 PDT
Created attachment 90480 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2011-04-20 20:18:32 PDT
The commit-queue encountered the following flaky tests while processing attachment 90480 [details]:

http/tests/xmlhttprequest/remember-bad-password.html bug 51733 (author: ap@webkit.org)
The commit-queue is continuing to process your patch.
Comment 7 WebKit Commit Bot 2011-04-20 20:21:42 PDT
Comment on attachment 90480 [details]
Patch for landing

Clearing flags on attachment: 90480

Committed r84460: <http://trac.webkit.org/changeset/84460>
Comment 8 WebKit Commit Bot 2011-04-20 20:21:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 WebKit Review Bot 2011-04-21 02:08:41 PDT
http://trac.webkit.org/changeset/84460 might have broken Qt Linux Release
Comment 10 Csaba Osztrogonác 2011-04-21 04:38:21 PDT
(In reply to comment #9)
> http://trac.webkit.org/changeset/84460 might have broken Qt Linux Release

http/tests/security/contentSecurityPolicy/frame-src-blocked.html 
broke http/tests/security/contentSecurityPolicy/image-allowed.html
on Qt bot, on Win7 release test bot and on Windows XP debug test bot

--- /home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/layout-test-results/http/tests/security/contentSecurityPolicy/image-allowed-expected.txt	2011-04-21 02:50:47.567542636 -0700
+++ /home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/layout-test-results/http/tests/security/contentSecurityPolicy/image-allowed-actual.txt	2011-04-21 02:50:47.567542636 -0700
@@ -1,2 +1,4 @@
+CONSOLE MESSAGE: line 1: Refused to load frame from 'http://127.0.0.1:8000/security/contentSecurityPolicy/resources/alert-fail.html' because of Content-Security-Policy.
+
 ALERT: PASS


--- /home/buildbot/slave/WebKit-BuildSlave/win-release-tests/build/layout-test-results/http/tests/security/contentSecurityPolicy/image-allowed-expected.txt	2011-04-21 03:36:29.043649200 -0700
+++ /home/buildbot/slave/WebKit-BuildSlave/win-release-tests/build/layout-test-results/http/tests/security/contentSecurityPolicy/image-allowed-actual.txt	2011-04-21 03:36:29.041649100 -0700
@@ -1,2 +1,6 @@
+CONSOLE MESSAGE: line 1: Refused to load frame from 'http://127.0.0.1:8000/security/contentSecurityPolicy/resources/alert-fail.html' because of Content-Security-Policy.
+
+CONSOLE MESSAGE: line 1: Refused to load frame from 'http://127.0.0.1:8000/security/contentSecurityPolicy/resources/alert-fail.html' because of Content-Security-Policy.
+
 ALERT: PASS
 

--- /home/buildbot/slave/win-debug-tests/build/layout-test-results/http/tests/security/contentSecurityPolicy/image-allowed-expected.txt	2011-04-21 19:26:48.328125000 -0700
+++ /home/buildbot/slave/win-debug-tests/build/layout-test-results/http/tests/security/contentSecurityPolicy/image-allowed-actual.txt	2011-04-21 19:26:48.328125000 -0700
@@ -1,2 +1,6 @@
+CONSOLE MESSAGE: line 1: Refused to load frame from 'http://127.0.0.1:8000/security/contentSecurityPolicy/resources/alert-fail.html' because of Content-Security-Policy.
+
+CONSOLE MESSAGE: line 1: Refused to load frame from 'http://127.0.0.1:8000/security/contentSecurityPolicy/resources/alert-fail.html' because of Content-Security-Policy.
+
 ALERT: PASS
Comment 11 Csaba Osztrogonác 2011-04-21 04:42:38 PDT
I tried to rollout but unfortunately it conflicts with r84478.
Comment 12 Adam Barth 2011-04-21 09:59:22 PDT
(In reply to comment #11)
> I tried to rollout but unfortunately it conflicts with r84478.

I've performed a partial rollout.  Sorry for the disruption.  :(
Comment 13 Adam Barth 2011-04-22 10:59:56 PDT
Created attachment 90725 [details]
Patch
Comment 14 Eric Seidel (no email) 2011-04-22 11:19:24 PDT
Comment on attachment 90725 [details]
Patch

We need to figure out how to unify these checks.  This ends up being a bunch of copy/paste code, which someone else editing this is likely to get wrong.
Comment 15 Adam Barth 2011-04-22 11:22:37 PDT
Comment on attachment 90725 [details]
Patch

I think we should have contentSecurityPolicy call canDisplay, which would simply all these call sites.
Comment 16 Eric Seidel (no email) 2011-04-22 11:26:07 PDT
(In reply to comment #15)
> (From update of attachment 90725 [details])
> I think we should have contentSecurityPolicy call canDisplay, which would simply all these call sites.

If you'd file the follow-up when you get the chance, that'd be fantastic.  Thanks again for the patch.
Comment 17 WebKit Commit Bot 2011-04-22 14:18:10 PDT
The commit-queue encountered the following flaky tests while processing attachment 90725 [details]:

http/tests/misc/favicon-loads-with-icon-loading-override.html bug 58412 (author: alice.liu@apple.com)
The commit-queue is continuing to process your patch.
Comment 18 WebKit Commit Bot 2011-04-22 14:20:29 PDT
Comment on attachment 90725 [details]
Patch

Clearing flags on attachment: 90725

Committed r84681: <http://trac.webkit.org/changeset/84681>
Comment 19 WebKit Commit Bot 2011-04-22 14:20:34 PDT
All reviewed patches have been landed.  Closing bug.
Comment 20 WebKit Commit Bot 2011-04-22 15:58:30 PDT
The commit-queue encountered the following flaky tests while processing attachment 90725 [details]:

http/tests/inspector/console-websocket-error.html bug 57392 (authors: pfeldman@chromium.org and yutak@chromium.org)
The commit-queue is continuing to process your patch.
Comment 21 WebKit Review Bot 2011-04-22 16:18:55 PDT
http://trac.webkit.org/changeset/84681 might have broken GTK Linux 64-bit Debug