68921 2011-09-27 11:44:02 -0700 CSP doesn't work for a wide variety of cases 2011-09-28 11:06:56 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED http://erlend.oftedal.no/blog/csp/readiness/latest.php P2 Normal --- 0 smus abarth abarth apf sam webkit.review.bot oldest_to_newest 474113 0 smus 2011-09-27 11:44:02 -0700 In my own tests, as well as in more comprehensive test suites, CSP policy is not being followed as expected. This test suite: http://erlend.oftedal.no/blog/csp/readiness/latest.php provides both X-Content-Security-Policy and X-WebKit-CSP headers, and uses the new syntax ('default-src' rather than 'allow'), but fails for most test cases. Do we have a CSP unit test suite? 474117 1 abarth 2011-09-27 11:48:36 -0700 > Do we have a CSP unit test suite? Our CSP tests are here: http://trac.webkit.org/browser/trunk/LayoutTests/http/tests/security/contentSecurityPolicy I'm not sure why we're doing so badly on the oftedal test suite. I'll investigate. 474455 2 sam 2011-09-27 20:32:32 -0700 This looks like an issue with our port matching code in CSPSource. In the case that the source's port is set to 0 (for no port specified) and the URL to match has no port specified (also set to 0), the code checks if 0 is the default port for the protocol. int port = url.port(); return port ? port == m_port : isDefaultPortForProtocol(m_port, url.protocol()); In this case, we should be returning true, something like this should probably work: bool portMatches(const KURL& url) const { if (m_portHasWildcard) return true; int port = url.port(); if (port == m_port) return true; if (!port) return isDefaultPortForProtocol(m_port, m_scheme); if (!m_port) return isDefaultPortForProtocol(port, m_scheme); return false; } 474457 3 sam 2011-09-27 20:36:23 -0700 Fixing that issue, it looks like we pass all the same tests that Firefox does. 474460 4 sam 2011-09-27 20:39:11 -0700 We probably have no tests for this case, since our testing harness requires using a non-default port :(. 474482 5 abarth 2011-09-27 21:33:55 -0700 Ah, that makes a lot of sense. Thanks Sam. Are you planning to post a patch, or should I? (Also, any thoughts on how we can test this bug?) 474826 6 109035 abarth 2011-09-28 10:42:01 -0700 Created attachment 109035 Patch 474848 7 109035 webkit.review.bot 2011-09-28 11:06:01 -0700 Comment on attachment 109035 Patch Clearing flags on attachment: 109035 Committed r96239: <http://trac.webkit.org/changeset/96239> 474849 8 webkit.review.bot 2011-09-28 11:06:06 -0700 All reviewed patches have been landed. Closing bug. 474854 9 abarth 2011-09-28 11:06:56 -0700 *** Bug 67008 has been marked as a duplicate of this bug. *** 109035 2011-09-28 10:42:01 -0700 2011-09-28 11:06:01 -0700 Patch bug-68921-20110928104200.patch text/plain 1753 abarth SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk2MjM0KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTkgQEAKKzIwMTEtMDktMjggIEFkYW0gQmFy dGggIDxhYmFydGhAd2Via2l0Lm9yZz4KKworICAgICAgICBDU1AgZG9lc24ndCB3b3JrIGZvciBh IHdpZGUgdmFyaWV0eSBvZiBjYXNlcworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9Njg5MjEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBQYXRjaCBzdWdnZXN0ZWQgYnkgU2FtIFdlaW5pZy4gIEl0J3MgdW5jbGVh ciB0byBtZSBob3cgdG8gdGVzdCB0aGlzCisgICAgICAgIGNoYW5nZSBiZWNhdXNlIGFsbCBvdXIg dGVzdHMgcnVuIG9uIG5vbi1kZWZhdWx0IHBvcnRzLCB3aGljaCBpcyB3aHkgd2UKKyAgICAgICAg aGF2ZSB0aGlzIGJ1ZyBpbiB0aGUgZmlyc3QgcGxhY2UuICBNb3ppbGxhIHVzZXMgYSBwcm94eSB3 aGlsZSB0ZXN0aW5nCisgICAgICAgIHNvIHRoZXkgY2FuIHRlc3Qgd2l0aCBVUkxzIGxpa2UgaHR0 cDovL2V4YW1wbGUuY29tLCBidXQgd2UgZG9uJ3QgaGF2ZQorICAgICAgICBzdWNoIGEgZmFjaWxp dHkuCisKKyAgICAgICAgKiBwYWdlL0NvbnRlbnRTZWN1cml0eVBvbGljeS5jcHA6CisgICAgICAg IChXZWJDb3JlOjpDU1BTb3VyY2U6OnBvcnRNYXRjaGVzKToKKwogMjAxMS0wOS0yOCAgRmFkeSBT YW11ZWwgIDxmc2FtdWVsQGNocm9taXVtLm9yZz4KIAogICAgICAgICBbQ2hyb21pdW1dIFNlcGVy YXRlIEdUSyBzcGVjaWZpYyBHeXAgcnVsZXMgZnJvbSBYMTEgR3lwIHJ1bGVzCkluZGV4OiBTb3Vy Y2UvV2ViQ29yZS9wYWdlL0NvbnRlbnRTZWN1cml0eVBvbGljeS5jcHAKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g U291cmNlL1dlYkNvcmUvcGFnZS9Db250ZW50U2VjdXJpdHlQb2xpY3kuY3BwCShyZXZpc2lvbiA5 NjIyOSkKKysrIFNvdXJjZS9XZWJDb3JlL3BhZ2UvQ29udGVudFNlY3VyaXR5UG9saWN5LmNwcAko d29ya2luZyBjb3B5KQpAQCAtMTQ3LDggKzE0NywxOSBAQCBwcml2YXRlOgogICAgIHsKICAgICAg ICAgaWYgKG1fcG9ydEhhc1dpbGRjYXJkKQogICAgICAgICAgICAgcmV0dXJuIHRydWU7CisKICAg ICAgICAgaW50IHBvcnQgPSB1cmwucG9ydCgpOwotICAgICAgICByZXR1cm4gcG9ydCA/IHBvcnQg PT0gbV9wb3J0IDogaXNEZWZhdWx0UG9ydEZvclByb3RvY29sKG1fcG9ydCwgdXJsLnByb3RvY29s KCkpOworCisgICAgICAgIGlmIChwb3J0ID09IG1fcG9ydCkKKyAgICAgICAgICAgIHJldHVybiB0 cnVlOworCisgICAgICAgIGlmICghcG9ydCkKKyAgICAgICAgICAgIHJldHVybiBpc0RlZmF1bHRQ b3J0Rm9yUHJvdG9jb2wobV9wb3J0LCBtX3NjaGVtZSk7CisKKyAgICAgICAgaWYgKCFtX3BvcnQp CisgICAgICAgICAgICByZXR1cm4gaXNEZWZhdWx0UG9ydEZvclByb3RvY29sKHBvcnQsIG1fc2No ZW1lKTsKKworICAgICAgICByZXR1cm4gZmFsc2U7CiAgICAgfQogCiAgICAgYm9vbCBpc1NjaGVt ZU9ubHkoKSBjb25zdCB7IHJldHVybiBtX2hvc3QuaXNFbXB0eSgpOyB9Cg==