Bug 68921 - CSP doesn't work for a wide variety of cases
Summary: CSP doesn't work for a wide variety of cases
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL: http://erlend.oftedal.no/blog/csp/rea...
Keywords:
: 67008 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-09-27 11:44 PDT by Boris Smus
Modified: 2011-09-28 11:06 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.71 KB, patch)
2011-09-28 10:42 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 Boris Smus 2011-09-27 11:44:02 PDT
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?
Comment 1 Adam Barth 2011-09-27 11:48:36 PDT
> 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.
Comment 2 Sam Weinig 2011-09-27 20:32:32 PDT
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;
    }
Comment 3 Sam Weinig 2011-09-27 20:36:23 PDT
Fixing that issue, it looks like we pass all the same tests that Firefox does.
Comment 4 Sam Weinig 2011-09-27 20:39:11 PDT
We probably have no tests for this case, since our testing harness requires using a non-default port :(.
Comment 5 Adam Barth 2011-09-27 21:33:55 PDT
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?)
Comment 6 Adam Barth 2011-09-28 10:42:01 PDT
Created attachment 109035 [details]
Patch
Comment 7 WebKit Review Bot 2011-09-28 11:06:01 PDT
Comment on attachment 109035 [details]
Patch

Clearing flags on attachment: 109035

Committed r96239: <http://trac.webkit.org/changeset/96239>
Comment 8 WebKit Review Bot 2011-09-28 11:06:06 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Adam Barth 2011-09-28 11:06:56 PDT
*** Bug 67008 has been marked as a duplicate of this bug. ***