Skip to content

Add support to avoid timing attacks in the csrf tokens #7221

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

Closed
wants to merge 1 commit into from

Conversation

eddumelendez
Copy link
Contributor

Fixes gh-4001

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 6, 2019
@rwinch
Copy link
Member

rwinch commented Aug 6, 2019

Thanks for the PR @eddumelendez!

This does not really solve the problem. We need to avoid the content of the page containing the same value on each request. As far as I can tell this does not solve the problem. The changes to CsrfToken and CsrfFilter look good, but I think we need to make some changes.

We need to update the DefaultCsrfToken getToken method to always return a different value. The value returned would be random bytes + xor(random bytes, original token value). The DefaultCsrfToken matches method would extract the random bytes (because it is a fixed length) and then xor it with the remaining bytes which calculates the original token value for comparison.

@rwinch rwinch added in: web An issue in web modules (web, webmvc) status: waiting-for-feedback We need additional information before we can continue type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 6, 2019
@eddumelendez
Copy link
Contributor Author

Thanks for the feedback @rwinch. I will be adding more commits in this PRs following your notes :)

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 6, 2019
@rwinch
Copy link
Member

rwinch commented Sep 30, 2019

Thanks for the PR @eddumelendez

gh-4001 is actually a different attack. It need what is written to the HTTP response to be random, but the CSRF token to be consistent. To do this we'd have the following:

  • actual_csrf_token (this is the value that we have now and is consistent throughout the session)
  • random_bytes - this is a secure random bytes Every time getToken is called we recalculate this value
  • xor_csrf_token is xor(random_bytes,actual_csrf_token)
  • result_csrf - This is random_bytes + xor_csrf_token

Then matches would see take the result_csrf:

  • extract random_bytes from result_csrf by length
  • extract xor_csrf_token from result_csrf by taking the remaining bytes
  • calculate actual_csrf_token by xor(random_bytes,xor_csrf_token)

Cheers,
Rob

@rwinch
Copy link
Member

rwinch commented Mar 4, 2020

Closing this since it is out dated and the feedback hasn't been applied

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSRF tokens are vulnerable to a BREACH attack
3 participants