Adding "style-src 'unsafe-inline' 'self'" to default CSP rules#41305
Adding "style-src 'unsafe-inline' 'self'" to default CSP rules#41305kobelb merged 12 commits intoelastic:masterfrom
Conversation
💔 Build Failed |
💔 Build Failed |
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
azasypkin
left a comment
There was a problem hiding this comment.
LGTM, tested locally (dashboards, visualizations, canvas, siem etc.) and haven't noticed anything that is broken because of this change. Just left one question and note
| `script-src 'unsafe-eval' 'nonce-{nonce}'`, | ||
| 'worker-src blob:', | ||
| 'child-src blob:', | ||
| `style-src 'unsafe-inline' 'self'`, |
There was a problem hiding this comment.
question: is there any reason we don't want to use nonce for inline styles as well?
There was a problem hiding this comment.
Using the nonce has caused quite a few issues, as discussed here and I'd like to find a way to move toward self even for script-src. I could see the argument being made for using the nonce until we switch script-src to self, but it's quite a bit more work which we'd rather quickly get rid of.
There was a problem hiding this comment.
Okay, I see, thanks for the explanation. Yeah, let's see how it goes then.
|
|
||
| const scriptSrc = parsed.get('script-src'); | ||
| expect(scriptSrc).to.be.an(Array); | ||
| expect(scriptSrc).not.to.contain('unsafe-inline'); |
There was a problem hiding this comment.
note: I'm a bit concerned about check like this. Our parsing logic in this test may not work as we expect at some point (even with packages like content-security-policy-parser) and we'll get false negatives here. Is there any reason we don't want to use expect().to.eql with a full list of "directives" here? Is it because of the nonce-some-unguessable-value? If so, can we filter startsWith('nonce-') out before asserting?
There was a problem hiding this comment.
Good point. I initially was trying to keep this test similar to how it was originally written, but you've made a convincing argument. Changes will be forth-coming.
💔 Build Failed |
💚 Build Succeeded |
💔 Build Failed |
💔 Build Failed |
|
retest |
💚 Build Succeeded |
…ic#41305) * Adding "style-src 'unsafe-inline' 'self'" to default CSP rules * Updating jest snapshot * Fixing api integration smoke test * Verifying all CSP responses * Fixing OIDC implicit flow test
…p-metrics-selectall * 'master' of github.com:elastic/kibana: (22 commits) [Code]: downgrade the log level of error message from subprocess (elastic#42925) [Code] Cancel clone/update job in the middle if disk space over the watermark (elastic#42890) Add Kibana App specific URL to the help menu (elastic#34739) (elastic#42580) [Maps] refactor createShapeFilterWithMeta to support more than just polygons (elastic#43042) Skip flaky es_ui_shared/request tests. Pass uiSettings to all data plugin services (elastic#42159) [SIEM] Upgrades react-redux and utilize React.memo for performance gains (elastic#43029) [skip-ci][Maps] add maki icon sheet to docs (elastic#43063) Adding "style-src 'unsafe-inline' 'self'" to default CSP rules (elastic#41305) Update dependency commander to v3 (elastic#43041) Update dependency @percy/agent to ^0.10.0 (elastic#40517) [Maps] only show top hits checkbox if index has date fields (elastic#43056) run chained_controls on Firefox to catch regression (elastic#43044) fixing issue with dashboard csv download (elastic#42964) Expose task manager as plugin instead of server argument (elastic#42966) Expose createRouter from HttpService, prepare handlers for context introduction (elastic#42686) [Code] disk watermark supports percentage and absolute modes (elastic#42987) [apps/dashboard] skip part of filtering tests on FF (elastic#43047) [ML] Kibana management jobs list (elastic#42570) [ML] Fix check for watcher being enabled (elastic#43025) ...
… (#43065) * Adding "style-src 'unsafe-inline' 'self'" to default CSP rules * Updating jest snapshot * Fixing api integration smoke test * Verifying all CSP responses * Fixing OIDC implicit flow test
💚 Build Succeeded |
Prior to this change, we didn't have any rules that applied to style elements. This change allows us to use
unsafe-inlinestyles, and styles that are loaded from "self".DevDocs
Adding
style-src 'unsafe-inline' 'self'to the default CSP rules