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

Configure the security middleware #5679

Merged
merged 1 commit into from May 9, 2019

Conversation

davidfischer
Copy link
Contributor

Configures the Django security middleware and the XFrameOptionsMiddleware which sets 3 headers on our HTTP responses served by Django.

  • The XSS_FILTER option will cause browsers that support it to implement a very basic XSS protection mechanism. Our site is probably not vulnerable to something this would prevent, but this is a best practice and may help prevent some future attacks against us
  • The NOSNIFF option helps prevent user uploaded files from being interpreted by a content type other than what the server sends. We don't serve any user uploaded files currently from Django (we do in nginx and we do in RTD corporate) so I don't see this being a problem. We probably aren't currently vulnerable to anything that this would prevent but again this helps future proof us.
  • We already set X-Frame-Options header in our nginx configs but this will help anyone with a private setup or in the event we forgot the header on an inherited block in nginx. This middleware and header help prevent clickjacking and basically prevents other sites from iframing RTD.

@davidfischer davidfischer requested a review from a team May 9, 2019 00:03
@humitos
Copy link
Member

humitos commented May 9, 2019

We don't serve any user uploaded files currently from Django (we do in nginx and we do in RTD corporate) so I don't see this being a problem.

I'm not sure this is 100% true. So, I'm commenting here just in case.

When serving files via Sendfile we set te Content-Type header by using mimetype.guess_type:

https://github.com/rtfd/readthedocs.org/blob/2e797fa1b45fe9772c6cf77c4fd55731d515585d/readthedocs/core/views/serve.py#L165-L170

Not sure this is a problem though, depending on which "guesser" is better in the end (Python module or browser). Anyway, I think that having this enabled is good for this case.

Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

These changes are good :)

@davidfischer
Copy link
Contributor Author

Not sure this is a problem though, depending on which "guesser" is better in the end (Python module or browser).

For the vast majority, I don't think it'll matter. If the file ends in .html, .css, .js it will get correctly interpreted.

@davidfischer davidfischer merged commit 7b93eac into master May 9, 2019
@delete-merged-branch delete-merged-branch bot deleted the davidfischer/security-middleware branch May 9, 2019 21:46
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

2 participants