Skip to content
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

Update Dockerfile-serve #519

Merged
merged 3 commits into from Aug 28, 2018
Merged

Update Dockerfile-serve #519

merged 3 commits into from Aug 28, 2018

Conversation

degeri
Copy link
Member

@degeri degeri commented Aug 21, 2018

This implements CSP.
It has the following rules.
Allows "self" , *.decred.org and *.dcrdata.org

This fixes #506

This implements CSP.
It has the following rules. 
Allows "self" , *.decred.org  and *.dcrdata.org


This fixes decred#506
@dajohi dajohi requested a review from jholdstock August 21, 2018 14:05
@jholdstock
Copy link
Member

Change looks fine for me. Just a couple of things to consider:

  • Are we adding *.decred.org and *.dcrdata.org because they are required right now? Or because we may require them in future? If the latter, I would suggest they are added when we actually need them, rather than adding them pre-emptively.
  • If I recall the production setup correctly, there is an nginx instance reverse proxying to multiple docker containers. Perhaps this kind of protection is actually better suited to live in the nginx instance? In fact, perhaps it is already there?

@degeri
Copy link
Member Author

degeri commented Aug 21, 2018

  1. Yes we need them now. We are loading content for the following.
    https://faucet.decred.org/requestfaucet request is made for purchasing credits in testnet and requests to *.dcrdata.org (testnet and explorer) are made for payment confirmation. We can go for a more granular rule like so (Content-Security-Policy "default-src 'self' testnet.dcrdata.org explorer.dcrdata.org testnet.decred.org mainnet.decred.org faucet.decred.org;) but I wanted to be safe and make sure nothing breaks.

  2. Yes I believe that other HTTP security headers come from nginx (X frame options,HSTS and XSS protection) but no CSP.

image

@fernandoabolafio
Copy link
Member

@dajohi can you take a quick look at this

default-src allows "self" , all subdomains of decred.org and dcrdata.org
img-src is set to "self" and "data:"
style-src is set to "self" and sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=' which is the hash of  (https://github.com/emotion-js/emotion/blob/master/packages/create-emotion/src/sheet.js#L52) only this inline style is allowed to run .
Copy link
Member

@fernandoabolafio fernandoabolafio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fernandoabolafio fernandoabolafio merged commit 955d0af into decred:master Aug 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implimenting CSP
3 participants