-
-
Notifications
You must be signed in to change notification settings - Fork 243
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
Main UI: Improve Content-Security-Policy #2714
Conversation
This new CSP is much more restrictive compared to the old one: - Don't allow any origin as default - Don't allow eval() usage - Don't allow data:, gap:, content: and blob: schemes by default Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
1613597
to
be7dd16
Compare
* allow loading media from any source, and data:, blob: and media: URIs | ||
* allow connecting (through fetch(), XMLHttpRequest, WebSocket etc.) to the same origin, raw.githubusercontent.com (add-on logos etc.), and any source | ||
--> | ||
<meta http-equiv="Content-Security-Policy" content="default-src 'self' 'unsafe-inline'; font-src 'self' data:; img-src * data:; media-src * data: blob: media:; connect-src 'self' raw.githubusercontent.com *;"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By removing this wildcard here we could limit what the HTTP action is able to do:
<meta http-equiv="Content-Security-Policy" content="default-src 'self' 'unsafe-inline'; font-src 'self' data:; img-src * data:; media-src * data: blob: media:; connect-src 'self' raw.githubusercontent.com *;"> | |
<meta http-equiv="Content-Security-Policy" content="default-src 'self' 'unsafe-inline'; font-src 'self' data:; img-src * data:; media-src * data: blob: media:; connect-src 'self' raw.githubusercontent.com;"> |
However we have to keep in mind that this limits the SIP widget and probably the video widget as well.
It is possible to overwrite our CSP here by using a reverse proxy, however is this quite user-unfriendly.
@ghys WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that this limits the SIP widget and probably the video widget as well
That's kind of bad then, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but as long as you have a reverse proxy in place this is easy to fix.
Either server everything from „self“, e.g. I have openhab.localnet for openHAB and my SIP WebSocket is at openhab.localnet/sip,
or overwrite the CSP by setting the header in the reverse proxy conf and add additional targets as needed.
#2194 Bundle Size — 10.82MiB (~+0.01%).d5eb72f(current) vs a13ffe3 main#2190(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
|
Current #2194 |
Baseline #2190 |
|
---|---|---|
Initial JS | 1.89MiB |
1.89MiB |
Initial CSS | 576.5KiB |
576.5KiB |
Cache Invalidation | 0.01% |
17.42% |
Chunks | 226 |
226 |
Assets | 249 |
249 |
Modules | 2914 |
2914 |
Duplicate Modules | 149 |
149 |
Duplicate Code | 1.8% |
1.8% |
Packages | 96 |
96 |
Duplicate Packages | 2 |
2 |
Bundle size by type 1 change
1 regression
Current #2194 |
Baseline #2190 |
|
---|---|---|
JS | 9.04MiB |
9.04MiB |
CSS | 862.88KiB |
862.88KiB |
Fonts | 526.1KiB |
526.1KiB |
Media | 295.6KiB |
295.6KiB |
IMG | 140.74KiB |
140.74KiB |
HTML | 1.33KiB (+7.55% ) |
1.24KiB |
Other | 871B |
871B |
Bundle analysis report Branch florian-h05:mainui-improve-csp Project dashboard
Generated by RelativeCI Documentation Report issue
@ghys I have tested this with my prod system by setting the header through my nginx, everything that is expected to work works fine. Can you please have a look? |
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested this with my prod system by setting the header through my nginx, everything that is expected to work works fine. Can you please have a look?
This would probably just confuse users unless properly documented ("you can only send HTTP requests to the server itself"), and probably be frustrating as well. I guess we'll just have to hope there won't be abuse. For marketplace widgets at least it's rather easy to delete them, at the minimum, if we discover they make malicious HTTP requests - this is explicitly banned in the Marketplace Rules anyways.
For changes targeting a specific instance/configuration however there's no good answer but then there are other ways (for example, with rules/rule templates).
Thanks!
Regression from #2714. eval() seems to be required by ECharts: When opening a chart, an CSP error is thrown that traces back to ECharts, but the chart still renders. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Regression from #2714. This updates Main UI‘s CSP to allow embedding any source. Fixes issues reported on the community, see https://community.openhab.org/t/openhab-4-3-milestone-discussion/158139/6 and https://community.openhab.org/t/openhab-4-3-milestone-discussion/158139/7. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
This new CSP is much more restrictive compared to the old one: