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

Add some security HTTP Headers #410

Merged
merged 3 commits into from
Dec 27, 2019

Conversation

elnappo
Copy link
Member

@elnappo elnappo commented Apr 8, 2019

Adds the following HTTP header (set by Django):

  • X-XSS-Protection
  • X-Content-Type-Option
  • Referrer-Policy

See #281

@elnappo
Copy link
Member Author

elnappo commented Apr 9, 2019

Added commit 9723a84 which sets HTTPONLY to CSRF cookies and USE_X_FORWARDED_HOST

@coveralls
Copy link

coveralls commented Apr 9, 2019

Coverage Status

Coverage increased (+0.3%) to 72.699% when pulling 60a3fe5 on elnappo:secure-http into 78616cd on nsupdate-info:master.

@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Apr 9, 2019

today i've updated the prod site to master branch (and also updated dependencies, but still dj 1.11).

if this is finished, we can merge it and try it soon.

@elnappo
Copy link
Member Author

elnappo commented Apr 10, 2019

Is ready to merge from my side. Should I add some additional header? CSP is rather complex and error prune. Which headers are currently added by nginx?

@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented May 14, 2019

nginx now:

1 add_header Strict-Transport-Security "max-age=31536000;";
2 add_header X-Content-Type-Options "nosniff";
3 add_header X-XSS-Protection "1; mode=block";
4 add_header Expect-CT "enforce, max-age=21600";
5 proxy_set_header Host $http_host;
6 proxy_set_header X-Real-IP $remote_addr;
7 proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
8 proxy_set_header X-Forwarded-Proto $scheme;

nginx after PR is merged:

1  add_header Strict-Transport-Security "max-age=31536000;";
2* -
3* -
4  add_header Expect-CT "enforce, max-age=21600";
5  proxy_set_header Host $http_host;
6  proxy_set_header X-Real-IP $remote_addr;
7  proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
8  proxy_set_header X-Forwarded-Proto $scheme;

@elnappo correct?

Considering 5, guess I don't want USE_X_FORWARDED_HOST = True.
Do you think this should be the default nevertheless?
Shall I rather change 5 into X-Forwarded-Host $http_host and use USE_X_FORWARDED_HOST = True?

@elnappo
Copy link
Member Author

elnappo commented May 16, 2019

Your'e right, we don't need X-Forwarded-Host. I removed it.

We also could add the Feature-Policy header:

proxy_set_header Feature-Policy "geolocation 'none'; midi 'none'; notifications 'none'; push 'none'; sync-xhr 'none'; microphone 'none'; camera 'none'; magnetometer 'none'; gyroscope 'none'; speaker 'none'; vibrate 'none'; fullscreen 'none'; payment 'none'; xr 'none'; usb 'none'; autoplay 'none'; legacy-image-formats 'none'; wake-lock 'none'";

https://github.com/w3c/webappsec-feature-policy
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Feature-Policy
https://scotthelme.co.uk/a-new-security-header-feature-policy/

@ThomasWaldmann
Copy link
Member

Our feature policy would read a bit like "fat free, sugar free, gluten free" printed on a bottle of mineral water.

But if it doesn't cause issues, why not have it...

@elnappo
Copy link
Member Author

elnappo commented May 18, 2019

Yes, unfortunately there is no "deny all" parameter...

@elnappo elnappo added this to the 1.0 release milestone Dec 27, 2019
@ThomasWaldmann ThomasWaldmann merged commit 5edb9f7 into nsupdate-info:master Dec 27, 2019
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.

None yet

3 participants