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

Fixed #30680 -- Removed obsolete system check for SECURE_BROWSER_XSS_FILTER setting. #11631

Merged
merged 1 commit into from Aug 7, 2019

Conversation

uadnan
Copy link
Contributor

@uadnan uadnan commented Aug 5, 2019

Removes security.W007 check as mentioned here https://code.djangoproject.com/ticket/30680

@felixxm felixxm changed the title Resolves #30680 -- Remove security.W007 check Fixed #30680 -- Removed obsolete system check for SECURE_BROWSER_XSS_FILTER setting. Aug 5, 2019
@felixxm felixxm self-assigned this Aug 5, 2019
Copy link
Sponsor Member

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

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

Thanks, great work. We’d also want a change log note. Perhaps: “The security.W007 check has been removed. It recommended enabling the setting SECURE_BROWSER_XSS_FILTER which sets the X-Xss-Protection header. This is no longer recommended as a security standard as many browsers have removed their XSS filters. You may still want to set the header if you support older browser versions.”

I’m not sure about which browsers have or have not removed their XSS filters (auditors). It might be worth researching just to make a slightly more accurate changleig note.

One last thing is maybe the documentation for the setting needs adjusting too with a note about the XSS auditors going out of style.

@felixxm
Copy link
Member

felixxm commented Aug 5, 2019

@adamchainz I don't think that we need to mention this in the release notes, but agreed we can add sth to a setting description, maybe:

Modern browsers don't honor ``X-XSS-Protection`` HTTP header anymore. Although
the setting offers little practical benefit, however you may still want to set
the header if you support older browsers.

@uadnan
Copy link
Contributor Author

uadnan commented Aug 5, 2019

I agree with @felixxm. Thanks @felixxm for changes

@MarkusH
Copy link
Member

MarkusH commented Aug 5, 2019

Should we add a information message to the checks framework that goes off when the settings variable is defined, to inform people that it changed / can be removed?

@felixxm
Copy link
Member

felixxm commented Aug 6, 2019

@MarkusH I don't think so, we removed security checks in the past without such steps (e.g. c27104a). Everyone should decide on their own we just do not recommend this anymore as a part of increasing security.

@adamchainz
Copy link
Sponsor Member

Agree with @felixxm - if a site has it defined already it's not harming them and may be required by their own security policies. Probably we'll need the setting for another 5 years still...

@felixxm
Copy link
Member

felixxm commented Aug 7, 2019

@uadnan Thanks!

@felixxm felixxm merged commit c507536 into django:master Aug 7, 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
4 participants