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

Support 'strict-dynamic' by default #1

Closed
mikispag opened this issue Nov 11, 2016 · 8 comments
Closed

Support 'strict-dynamic' by default #1

mikispag opened this issue Nov 11, 2016 · 8 comments

Comments

@mikispag
Copy link

It would be really cool if you could add support for 'strict-dynamic' (W3C) by default.

Google is offering Google Patch Rewards for integrating CSP in popular frameworks and facilitating adoption (as you already did, so please apply!).

The most recent research on CSP demonstrated that the whitelist-based approach is intrinsically flawed for XSS mitigation, and that a nonce/hash + 'strict-dynamic' policy, similar to the ones already served by important Google products, is in the quasi-totality of cases the only working solution.

There are a lot of misconceptions about CSP, and getting rid of things that are proven not to work is good - the worst thing you can have is a false sense of security.

If you could also add a link to the CSP Evaluator to your README, to let users evaluate the quality of their policy, it would be awesome.

Please do not hesitate to get back to me if you need any additional information or have doubts.

More resources

@aidantwoods
Copy link
Owner

aidantwoods commented Nov 11, 2016

@mikispag

Google is offering Google Patch Rewards for integrating CSP in popular frameworks and facilitating adoption (as you already did, so please apply!).

I'll definitely take a look at this, thanks for the link! Is it worth applying now, or should I wait until I have a proper full version done? (shouldn't be long away!)

The most recent research on CSP demonstrated that the whitelist-based approach is intrinsically flawed for XSS mitigation, and that a nonce/hash + 'strict-dynamic' policy, similar to the ones already served by important Google products, is in the quasi-totality of cases the only working solution.

Don't have to convince me of that! 😉

RE making it default behaviour – at the moment my approach towards pushing good settings is having SecureHeaders 'complain' until those settings are configured ('sanely').

With that in mind, adding a check for 'strict-dynamic' in the CSP sanity check would be most inline with SecureHeaders' current behaviour (useful but hopefully non intrusive defaults, and compain until things are ideal).

I.e. so that 'strict-dynamic' is encouraged (if a source list is present) similarly to these being discouraged https://github.com/aidantwoods/SecureHeaders#another-example

the worst thing you can have is a false sense of security

This comment is along the lines of why I wouldn't want to specify a default CSP for SecureHeaders – it would either break things, or be overly broad and useless. Though I can see a good middle ground where a dev could opt-in to some defaults that are 'better, but may need configuring to make your application work'.

Alternatively, I could auto-add 'strict-dynamic' if a nonce is in the source list, but not otherwise. (though automatic behaviour like this needs to be balanced imo – if it's too unintuative, it may discourage use of SecureHeaders all together if devs find it unpredictable. Especially if devs are testing in non CSP3 compliant browsers – they wouldn't even notice it).

If you could also add a link to the CSP Evaluator to your README, to let users evaluate the quality of their policy, it would be awesome.

I'll definately add a link to that – it's a great resource. It may make sense to add the link into warnings triggered by the CSP sanity check too, just so devs see it if they're writing CSPs that raise obvious flags.

@aidantwoods aidantwoods self-assigned this Nov 11, 2016
aidantwoods added a commit that referenced this issue Nov 13, 2016
… implementation of issue #1), and auto-deploys HSTS with long duration, subdomains and preload
aidantwoods added a commit that referenced this issue Nov 13, 2016
… implementation of issue #1), and auto-deploys HSTS with long duration, subdomains and preload
aidantwoods added a commit that referenced this issue Nov 13, 2016
…o hsts can be gated with header_proposals (like exisiting auto-added headers) ref: #1
@aidantwoods
Copy link
Owner

Okay, so I've added ->strict_mode() which bundles 'strict-dynamic' in along with HSTS with long duration and preload criteria.

SecureHeaders will honour user set HSTS preferences if set though (or if they've asked to remove the HSTS header via ->remove_header()).

I'll probably think of a few other interesting things to stick in 'strict-mode', could possibly make it add a few extensions to safe-mode, e.g. wildcards picked up in certain directives during CSP validation are auto-removed.

@mikispag
Copy link
Author

Very nice, thanks!

Do you enforce the presence of at least one nonce or hash together with 'strict-dynamic'? Otherwise it will just break the application...

@aidantwoods
Copy link
Owner

I was thinking more along the lines of spitting out an E_USER_WARNING if no nonces/hashes are added. Since ->strict_mode() is opt-in, IMO it's valuable to show devs what settings they should use in headers, and encourage them to change their application around those (rather than hoping they happen to generate a nonce/hash to benefit).

Let me know if that makes sense? Happy to reconsider it if I'm making a silly decision here

@mikispag
Copy link
Author

I think we should not add 'strict-dynamic' if there is no nonce or hash, because it would just break the application. So we should probably enforce the use of nonces or hashes in strict mode, together with 'strict-dynamic'. What do you think?

@aidantwoods
Copy link
Owner

Summary of that change:

If strict mode is enabled, SecureHeaders will take directive to be either default-src or script-src (whichever is set). If both are set, script-src is used. If directive contains a nonce or hash source value then inject 'strict-dynamic' into the CSP source list for directive, otherwise issue the following:

Warning: Strict Mode is enabled, but couldn't add 'strict-dynamic' into the Content-Security-Policy because no hash or nonce was used.

@mikispag
Copy link
Author

Awesome, exactly what I was thinking, thanks!

@aidantwoods
Copy link
Owner

Great 👍 Let me know if you think of anything else!

@aidantwoods aidantwoods changed the title [Feature Request] Support 'strict-dynamic' by default Support 'strict-dynamic' by default Jul 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants