Make X-Xss-Protection header value configurable in ServerHttpSecurity #11908
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spring-projects-issues
added
the
status: waiting-for-triage
Kehrlann
force-pushed
the
5.8/x-xss-protection
branch
3 times, most recently
from
f423809 to
809dc96
Compare
sjohnr
reviewed
Sep 27, 2022
...n/java/org/springframework/security/config/annotation/web/configurers/HeadersConfigurer.java
Show resolved
Hide resolved
Kehrlann
force-pushed
the
5.8/x-xss-protection
branch
from
809dc96 to
f9c2ae2
Compare
sjohnr
reviewed
Sep 28, 2022
There was a problem hiding this comment.
Thanks @Kehrlann!
Should we plan to align the Reactive header with the Servlet version?
While I don't know the answer off-hand, shall we create a separate issue to track this? You can label it with for: team-attention and ask about it at our next standup if needed.
You may know the process already, but in case it's new:
When merging this, you can merge into 5.8.x, then immediately merge 5.8.x into main. Afterwards, you would want to update whats-new.adoc in 5.8.x and then git merge -s ours 5.8.x into main so that What's New is only updated in 5.8.
Let us know if you would like another review before merging. My review below is mostly cosmetic.
config/src/main/kotlin/org/springframework/security/config/web/server/ServerXssProtectionDsl.kt
Show resolved
Hide resolved
...ain/kotlin/org/springframework/security/config/web/servlet/headers/XssProtectionConfigDsl.kt
Show resolved
Hide resolved
...rc/main/java/org/springframework/security/web/header/writers/XXssProtectionHeaderWriter.java
Show resolved
Hide resolved
...va/org/springframework/security/web/server/header/XXssProtectionServerHttpHeadersWriter.java
Show resolved
Hide resolved
Kehrlann
force-pushed
the
5.8/x-xss-protection
branch
2 times, most recently
from
4da0bd8 to
57bc437
Compare
OWASP recommends using "X-Xss-Protection: 0". The default is currently "X-Xss-Protection: 1; mode=block". In 6.0, the default will be "0". This commits adds the ability to configure the xssProtection header value in ServerHttpSecurity. This commit deprecates the use of "enabled" and "block" booleans to configure XSS protection, as the state "!enabled + block" is invalid. This impacts HttpSecurity. Issue spring-projectsgh-9631
Kehrlann
force-pushed
the
5.8/x-xss-protection
branch
from
57bc437 to
a5b23e6
Compare
|
Merged via 9325001 |
sjohnr
added
in: web
|
Note that we will want to update What's New with this change as a separate commit in 5.8. |
This was referenced Oct 3, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make
X-Xss-Protectionheader configurable inServerHttpSecurity, re issue: gh-9631.XSS Protection is configurable through
ServerHttpSecurity#xssProtection#headerValue, with aHeaderValueenum (DISABLED / ENABLED / ENABLED_MODE_BLOCK). This is because the state!enabled && blockis invalid, thus we restrict the API to the three valid states only.I ported the changes over to the Servlet stack, which has more configuration exposed in
HttpSecurity, so the deprecations bubble up to that class too.The idea in 6.0 is to change the default to
HeaderValue = DISABLED, and also remove theenabledandblockbooleans.As part of this change, I noticed that in servlet the "mode=block" header values are different in the Servlet and Reactive stacks:
X-Xss-Protection: 1; mode=blockin ServletX-Xss-Protection: 1 ; mode=blockin Reactive, with an extra spaceShould we plan to align the Reactive header with the Servlet version?