BREACH attack protection for CSRF tokens #10778
Conversation
sjohnr
added
status: duplicate
web/src/main/java/org/springframework/security/web/csrf/XorCsrfTokenRepository.java
Outdated
Show resolved
Hide resolved
web/src/main/java/org/springframework/security/web/csrf/XorCsrfTokenRepository.java
Outdated
Show resolved
Hide resolved
web/src/test/java/org/springframework/security/web/csrf/XorCsrfTokenRepositoryTests.java
Outdated
Show resolved
Hide resolved
sjohnr
force-pushed
the
gh-4001
branch
2 times, most recently
from
67179e2 to
f32661c
Compare
1. added new XorCsrfToken class 2. introduce new method setGenerateToken for CookieCsrfTokenRepository and HttpSessionCsrfTokenRepository to customize CsrfToken implementation 3. deprecate `setHeaderName` and `setParameterName` Closes spring-projectsgh-4001 Co-Authored-By: Rob Winch <[email protected]>
There was a problem hiding this comment.
We have spoken offline and are going to explore some alternatives.
Now that it is implemented, something that doesn't sit well is the fact that the repository is performing logic other than persistence. I'm wondering if we need to put the logic of reading the request and writing the request attributes into another API that can do the secure random. This also has the advantage of not impacting persistence and only changing the token a single time per request.
We will explore if https://github.com/spring-projects/spring-security/blob/6dbd88a5a4857c279aed681529[…]main/java/org/springframework/security/web/csrf/CsrfFilter.java
and
https://github.com/spring-projects/spring-security/blob/6dbd88a5a4857c279aed681529[…]main/java/org/springframework/security/web/csrf/CsrfFilter.java
were placed into a new API.
@sjohnr has agreed to a branches that demonstrate the ideas.
|
@rwinch I have pushed a branch (draft 1) based on our discussion, and looking for general feedback on the approach. This draft uses functional hooks. The next approach we could try would be to introduce a new interface. I don't have a good idea for what it would be called but let me know if you would like to see that approach next, or if you have another idea for the next branch. |
|
I think this is a good start. I agree with you that we should create specific interfaces. An interesting observation is that CsrfAuthenticationStrategy can reuse the logic for setting the CSRF on an attribute. I think that the names you have used could be used as a valid interface names. RequestAttributeHandler and CsrfTokenRequestResolver (we might need to play with this a bit). We should have a single class implement both APIs since they are distinct jobs, but need to be coordinated. |
|
Replaced by gh-11731 |
No description provided.