Remove cases of `unsafe` scripts or styles, along with remote sources for images

Gunni commented 1 year ago

Checklist

[X] I have updated to the latest available Home Assistant version.
[X] I have cleared the cache of my browser.
[X] I have tried a different browser to see if it is related to my browser.

Describe the issue you are experiencing

I was writing a Content-Security-Policy for my Home Assistant. Note that my instance runs behind a reverse proxy with a trusted https certificate. The policy I ended up with after a lot of testing is:

default-src 'self';
script-src 'self' 'unsafe-inline' 'unsafe-eval';
style-src 'self' 'unsafe-inline';
img-src 'self' data: https://brands.home-assistant.io https://basemaps.cartocdn.com;
font-src 'self' data:;
frame-ancestors 'self';
form-action 'self';
upgrade-insecure-requests;
block-all-mixed-content;
sandbox allow-forms allow-same-origin allow-scripts allow-popups;
base-uri 'none'"

Describe the behavior you expected

I would expect good html practices to be used, where unsafe inline code is avoided and moved to script/style files.

I also expect HA to only use local resources since we want everything (reasonable, obv not remote APIs) to work even if we are offline, and not reference remote sources such as https://brands.home-assistant.io and https://basemaps.cartocdn.com.

Steps to reproduce the issue

Add reverse proxy in front of HA, with https and a trusted certificate
Add the above CSP
Try removing some unsafes or image-srcs such, observe what breaks

What version of Home Assistant Core has the issue?

2023.2.4

What was the last working version of Home Assistant Core?

No response

In which browser are you experiencing the issue with?

Firefox 109.0.1 (64-bit)

Which operating system are you using to run this browser?

Windows 10

State of relevant entities

No response

Problem-relevant frontend configuration

No response

Javascript errors shown in your browser console/inspector

No response

Additional information

No response

steverep commented 1 year ago

I would expect good html practices to be used, where unsafe inline code is avoided and moved to script/style files.

Regarding styles, HA uses web components and shadow DOM heavily, which relies on inline styles or constructable stylesheets for encapsulation. Until the latter is widely supported, inline styles will continue to exist. That said, a large chunk of them could probably easily be nonced.

AFAIK there are very few inline scripts, so these would have to be addressed one by one.

I also expect HA to only use local resources since we want everything (reasonable, obv not remote APIs) to work even if we are offline, and not reference remote sources such as https://brands.home-assistant.io and https://basemaps.cartocdn.com.

Making those resources local is certainly unreasonable.

The first hosts a very large collection of brand images to associate with integrations and devices. Bundling them with every HA install would bloat and slow things down unnecessarily for everyone, especially since the average instance will only use a small percentage of them.
The second is the base images for maps. I think it's obvious that shouldn't be a local resource.

steverep commented 1 year ago

@bramkragten as a "catch most" strategy, we should set window.litNonce.

Gunni commented 1 year ago

The first hosts a very large collection of brand images to associate with integrations and devices. Bundling them with every HA install would bloat and slow things down unnecessarily for everyone, especially since the average instance will only use a small percentage of them.

While I agree with you, we are only talking about ~200 MB here.

Maps

Yes, fine, agreed

steverep commented 1 year ago

While I agree with you, we are only talking about ~200 MB here.

That's plenty large enough not to send them all to the browser, which means every HA instance would need to host the equivalent of brands.home-assistant.io and incur all the associated overhead.

bramkragten commented 1 year ago

@bramkragten as a "catch most" strategy, we should set window.litNonce.

Yes, we can do that

steverep commented 1 year ago

That would cover most inline style tags, and also setting __webpack_nonce__ would inject the nonce on any script tags.

I think we'd just have to create the nonce in core and substitute it into the HTML templates. I'm not sure the best way to do that part though.

Gunni commented 1 year ago

While I agree with you, we are only talking about ~200 MB here.

That's plenty large enough not to send them all to the browser, which means every HA instance would need to host the equivalent of brands.home-assistant.io and incur all the associated overhead.

Fine, what about taking the most commonly used ones and adding it to HA?

Or, act more intelligent, when an icon is used and we are online, cache it locally/forever for future use. From that point onward that icon is always local.

Forever feels fine because if they load an icon, they needed it at some point anyway.

steverep commented 1 year ago

Your browser is already going to cache the ones you use, and neither of those approaches would make the domain removable from the CSP, so I think we're digressing here. Having a few trusted sources of remote images should not be a big security concern.

Inline scripts and styles on the other hand, I agree we should work to secure as they pose a much more significant XSS concern. Adding nonce attributes and putting that in the CSP is the right approach.

Gunni commented 1 year ago

Your browser is already going to cache the ones you use

Only that specific browser on that specific device might.

neither of those approaches would make the domain removable from the CSP

That's why you proxy the request via a script on the server that takes care of the caching I mentioned. Added bonus is that all the user requests get anonymized by always coming from the server IP.

Having a few trusted sources of remote images should not be a big security concern.

My concern is also that it does not work if I am offline.

Inline scripts and styles on the other hand, I agree we should work to secure as they pose a much more significant XSS concern. Adding nonce attributes and putting that in the CSP is the right approach.

💖

github-actions[bot] commented 1 year ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

steverep commented 1 year ago

Still relevant

Codel1417 commented 1 year ago

Maybe an addon could exist which serves the images & periodically pulls the GitHub repo for changes.

An option would need to exist in Home Assistant to set the external image host.

github-actions[bot] commented 11 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

cyberhuman commented 11 months ago

Still interested.

github-actions[bot] commented 8 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

Gunni commented 8 months ago

This is still a problem.

Also see https://github.com/home-assistant/frontend/issues/18549#issuecomment-1859080426

github-actions[bot] commented 5 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

Gunni commented 5 months ago

This is still a problem.

mwdle commented 3 months ago

+1

github-actions[bot] commented 3 weeks ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

Gunni commented 3 weeks ago

Still relevant

home-assistant / frontend