Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Content Security Policy Header does not respect IE 10/11 behaviour #3770

Closed
jgoldhammer opened this issue Mar 24, 2016 · 7 comments
Closed
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Milestone

Comments

@jgoldhammer
Copy link
Contributor

I am using Spring Security 4.0 in a bigger project and have written my own Content Security Policy Header Writer which writes different header values depending on the user agent delivered in the http request.

I am using https://github.com/HaraldWalker/user-agent-utils to detect the browser and version.
For Example, for IE10/11 only X-Content-Security-Policy with the sandbox attribute are allowed to use...

Reference Spring Security 4.1:

response.setHeader((!reportOnly ? CONTENT_SECURITY_POLICY_HEADER : CONTENT_SECURITY_POLICY_REPORT_ONLY_HEADER), policyDirectives);

Do you think it is worth to make a pull request?

@jgrandja
Copy link
Contributor

Hi Jens,

Thanks for the feedback!
And yes you are correct, that 'sandbox' directive takes effect only on the Content-Security-Policy header. It has no effect on the Content-Security-Policy-Report-Only header.

As per the W3C Candidate Recommendation in the section 7.14. sandbox:

"The sandbox directive will be ignored when monitoring a policy, and when contained in a policy defined via a meta element. Moreover, this directive has no effect when monitored, and has no reporting requirements."

FYI, the initial implementation of Content Security Policy in Spring Security is fairly simple. The reason for this is that the proposed specification is still evolving and there may be more changes coming before the specification is finalized. So at this point, the API provides full control to the web application author in configuring the policy directive(s) for the intended CSP header.

For example, given this configuration:

httpSecurity
.headers()
.contentSecurityPolicy("sandbox").reportOnly();

this would write the following header:

Content-Security-Policy-Report-Only: sandbox

As per the spec, this would be ignored by the user-agent.

As the spec evolves and ultimately finalizes, we may add a SecurityPolicyBuilder abstraction that would validate against such an 'invalid' or 'ignored' configuration.

So for now, my recommendation would be to ensure the correct policy directives are set for the configured header.

Does this make sense?

@jgoldhammer
Copy link
Contributor Author

I can definitely understand your comment. Nevertheless the current situation differs. If you want to support IE11/Edge, the situation is different.

The header name differs for that browsers. IE only supports X-Content-Security-Policy, the other browsers Content-Security-Policy...

So that difference in not covered by the current implementation...

@rwinch rwinch added the in: web An issue in web modules (web, webmvc) label Mar 25, 2016
@rwinch rwinch added this to the 4.1.0 milestone Mar 25, 2016
@rwinch rwinch assigned rwinch and jgrandja and unassigned rwinch Mar 25, 2016
@rwinch
Copy link
Member

rwinch commented Mar 25, 2016

@jgoldhammer Thanks for the feedback! I see your point. We will provide an update in the latest release to also include X-Content-Security-Policy.

@rwinch rwinch added the type: enhancement A general enhancement label Mar 25, 2016
@jgrandja
Copy link
Contributor

@jgoldhammer I have to apologize as I misread your initial comment. I thought you were referring to the new standard header name proposed by the W3C Content-Security-Policy. However, you were actually referring to the deprecated, experimental header X-Content-Security-Policy that is supported by IE 10/11.
No worries, as @rwinch has already mentioned, we will update the latest release to also include the X-Content-Security-Policy header as well.
Thanks for pointing that out!

@jgrandja
Copy link
Contributor

@jgoldhammer I’ve been looking at various sources on the current use of the experimental/deprecated header X-Content-Security-Policy. The following site provides a list of all browser versions that provide support (or don’t) for Content Security Policy for Level 1 and 2.

http://caniuse.com/#search=content%20security%20policy

You will notice in the support chart that IE 10 & 11 provide partial support through the deprecated header X-Content-Security-Policy, as we have already noted. However, IE/Edge 13 & 14 do provide support for the new standard header Content-Security-Policy. Not to mention most other browsers provide support for the standard header as well.

Also note from the chart, that the browser usage for IE 10 & 11 is quite low.

Browser support for CSP Level 2 is not quite there yet as it introduced a few new directives that browser’s haven’t implemented as of yet.

I did some testing (using different IE versions) with some of the more popular sites that are currently using CSP.

Facebook
IE 13 - sends back Content-Security-Policy
IE 10, 11 - does not send X-Content-Security-Policy or Content-Security-Policy

Twitter
IE 13 - sends back Content-Security-Policy
IE 11 - does not send X-Content-Security-Policy or Content-Security-Policy

Github
IE 10, 11, 13 - sends back Content-Security-Policy only

I haven’t been able to find a site that is sending back the experimental/deprecated header X-Content-Security-Policy.

Also, if you take a look at the following link: http://content-security-policy.com/

You will notice the following content:

Note: It is known that having both Content-Security-Policy and X-Content-Security-Policy or X-Webkit-CSP causes unexpected behaviours on certain versions of browsers. Please avoid using deprecated X-* headers.

I have discussed these findings with @rwinch and we don’t think adding support for X-Content-Security-Policy is appropriate for the framework.

What I would like to propose for your use case is to use the Static Headers feature in Spring Security http://docs.spring.io/spring-security/site/docs/4.1.0.RC1/reference/htmlsingle/#headers-static

Using this feature will allow you to support any custom header, specifically, the X-Content-Security-Policy.

Here is a Xml configuration example:

<http>
    <!-- ... -->

    <headers>
        <header name="X-Content-Security-Policy" value=“sandbox” />
    </headers>
</http>

Or using Java configuration:

@EnableWebSecurity
public class WebSecurityConfig extends
WebSecurityConfigurerAdapter {

@Override
protected void configure(HttpSecurity http) throws Exception {
    http
    // ...
    .headers()
        .addHeaderWriter(new StaticHeadersWriter("X-Content-Security-Policy”,”sandbox”));
}
}

@jgoldhammer
Copy link
Contributor Author

@jgrandja Thanks for your detailed answer.

"Also note from the chart, that the browser usage for IE 10 & 11 is quite low."
You don´t know big german enterprises :-). One of our customer uses IE10 and will migrate to IE11 this year. Another customer used IE9 till end of last year...

Thanks for pointing out the Staticheaderwriter which is no solution when you want to support different browsers and want to differentiate between IE/Edge and the rest of the browsers.

So as I have pointed out I have my own solution and I only wanted to know if we should provide a PR for that. As this is not of interest, no problem for me. I hope, I can contribute in another way to your project...

Thanks,
Jens

@jgrandja
Copy link
Contributor

@jgoldhammer We really appreciate your feedback and pointing things out regarding this issue. And we would welcome any contributions you may have. Looking forward to hearing from you again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants