Don't add csp-headers if none are required #21318

Merged
merged 1 commit into from Jan 17, 2017

Projects

None yet

5 participants

@arjenm
Contributor
arjenm commented Jan 17, 2017 edited
Q A
Branch? 3.2
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets This PR is also the ticket
License MIT

In 3.2 a tool to adjust Content Security Policy headers in combination with the WebProfiler was added. We encountered a bug in its behavior.
We had CSP-headers that did not have a script-src/style-src nor a default-src (it was something like form-action: https:). In that scenario, the ContentSecurityPolicyHandler would add script-src: 'unsafe-inline' 'nonce-....', but that would actually change the "everything is allowed scenario" into "only inline and nonce-... is allowed". The result was only the javascript of WebProfiler was allowed, rather than everything.

This PR fixes the scenario where no default-src nor a script-src/style-src is provided. It simply continue's rather than treats it as an empty list of rules that need additional rules.

A bug I did find, but not fix, is the fact that that 'unsafe-inline' is ignored in at least Firefox and Chrome due to the fact there is also a nonce-element in the rule.

@arjenm arjenm Don't add csp-headers if none are required
6fecc94
@stof
Member
stof commented Jan 17, 2017

A bug I did find, but not fix, is the fact that that 'unsafe-inline' is ignored in at least Firefox and Chrome due to the fact there is also a nonce-element in the rule.

This is not a bug. unsafe-inline combined with nonces is about supporting both CSP 1 and CSP 2 together

@arjenm
Contributor
arjenm commented Jan 17, 2017

Ok, it doesn't really matter for my PR anyway, so I scratched that. Thanks for correcting me :)

@romainneutron
Member

LGTM 👍

@fabpot
Member
fabpot commented Jan 17, 2017

Thank you @arjenm.

@fabpot fabpot merged commit 6fecc94 into symfony:3.2 Jan 17, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
@fabpot fabpot added a commit that referenced this pull request Jan 17, 2017
@fabpot fabpot bug #21318 Don't add csp-headers if none are required (arjenm)
This PR was merged into the 3.2 branch.

Discussion
----------

Don't add csp-headers if none are required

| Q             | A
| ------------- | ---
| Branch?       | 3.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | This PR is also the ticket
| License       | MIT

In 3.2 a tool to adjust Content Security Policy headers in combination with the WebProfiler was added. We encountered a bug in its behavior.
We had CSP-headers that did not have a script-src/style-src nor a default-src (it was something like `form-action: https:`). In that scenario, the ContentSecurityPolicyHandler would add `script-src: 'unsafe-inline' 'nonce-....'`, but that would actually change the "everything is allowed scenario" into "only inline and nonce-... is allowed". The result was _only_ the javascript of WebProfiler was allowed, rather than everything.

This PR fixes the scenario where no default-src nor a script-src/style-src is provided. It simply continue's rather than treats it as an empty list of rules that need additional rules.

~A bug I did find, but not fix, is the fact that that `'unsafe-inline'` is ignored in at least Firefox and Chrome due to the fact there is also a nonce-element in the rule.~

Commits
-------

6fecc94 Don't add csp-headers if none are required
84d0da2
@fabpot fabpot referenced this pull request Feb 6, 2017
Merged

Release v3.2.3 #21544

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment