-
Notifications
You must be signed in to change notification settings - Fork 898
feat: improve security of electron app #741
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
Conversation
License: MIT Signed-off-by: Henrique Dias <[email protected]>
License: MIT Signed-off-by: Henrique Dias <[email protected]>
olizilla
left a comment
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.
Near PR. Need to dig into what the correct CSP is for web ui. That could be implemented as a meta tag and PR'd on webui too.
src/hooks/app-events.js
Outdated
| callback({ // eslint-disable-line | ||
| responseHeaders: { | ||
| ...details.responseHeaders, | ||
| 'Content-Security-Policy': ['default-src \'none\''] |
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.
How did you come to pick this as the CSP? I just ran the CSP Labratory plugin on Web UI and it came up with
default-src 'none'; connect-src 'self' http://127.0.0.1:5001/api/v0/config/show http://127.0.0.1:5001/api/v0/get http://127.0.0.1:5001/api/v0/id http://127.0.0.1:5001/api/v0/object/get http://127.0.0.1:5001/api/v0/stats/bw http://127.0.0.1:5001/api/v0/swarm/peers; font-src 'self'; img-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'
which i think we can trim down, but it's a start.
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.
@olizilla maybe it could be trimmed down to:
default-src 'none'; connect-src 'self'; font-src 'self'; img-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'
I set it to default-src none because it was the less permissive of all and it worked. Which now leads me to think that disabling web security might disable CSP. I couldn't find any info about it. But it is in fact weird that setting the default-src to none doesn't change anything. I'll investigate.
License: MIT Signed-off-by: Henrique Dias <[email protected]>
|
@olizilla just updated it here. |
|
were you able to verify if it is applied when webSecurity is false? |
Prevent any URL that is not from our app to open inside one of our windows. This takes into account the recommendations of Electron's team.
Update: content security policy headers seem not to work when web security is set to false.