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

CSRF tokens are vulnerable to a BREACH attack #4001

Closed
johnraycp opened this issue Aug 2, 2016 · 8 comments
Closed

CSRF tokens are vulnerable to a BREACH attack #4001

johnraycp opened this issue Aug 2, 2016 · 8 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Milestone

Comments

@johnraycp
Copy link

Summary

The CSRF tokens generated by Spring are vulnerable to a BREACH attack. More details at http://breachattack.com/

I'll create a pull request with a proposed code change to fix this.

Actual Behavior

Spring always returns the same CSRF token to the browser.

Expected Behavior

The simplest way to mitigate this would be to return a token which is composed of a random per secret request XORed with an internal CSRF token. This effectively means that the browser receives a new CSRF token with each request.

Configuration

This only occurs when you turn on CSRF protection in Spring and also have HTTP compression enabled somewhere in your web server stack.

Version

Currently exists in latest version in Git.

Sample

N/A

@jgrandja
Copy link
Contributor

jgrandja commented Sep 2, 2016

The proposed solution will be to introduce a default method to CsrfToken, as follows:

public interface CsrfToken {
    …
    default boolean matches(CsrfToken token) {
        return getToken().equals(token.getToken());
    }
}

Then override the default method in DefaultCsrfToken to handle the XOR logic.

Targeted for 5.0 RELEASE which will support Java 8.

@jgrandja jgrandja added in: web An issue in web modules (web, webmvc) New Feature labels Sep 2, 2016
@jgrandja jgrandja removed their assignment Sep 2, 2016
@rwinch rwinch modified the milestones: 5.0.0.M2, 5.0.0.M1 May 9, 2017
@rwinch rwinch modified the milestones: 5.0.0.M2, 5.0.0.M3 Jun 15, 2017
@jgrandja jgrandja modified the milestones: 5.0.0.M3, 5.0.0.M4 Jul 24, 2017
@rwinch rwinch modified the milestones: 5.0.0.M4, 5.0.0.M5 Sep 13, 2017
@rwinch rwinch modified the milestones: 5.0.0.M5, 5.0.0.RC1 Oct 3, 2017
@rwinch rwinch modified the milestones: 5.0.0.RC1, 5.0.0 Oct 30, 2017
@Wadeck
Copy link

Wadeck commented Nov 24, 2017

@jgrandja care about potential timing-attack with your proposal. Using MessageDigest.isEqual could be better at that point.

@rwinch rwinch modified the milestones: 5.0.0, 5.1.0.M1 Nov 27, 2017
@rwinch rwinch modified the milestones: 5.1.0.M1, 5.1.0.RC1 Dec 19, 2017
@rwinch rwinch modified the milestones: 5.1.0.M2, 5.1.0.RC1 Jul 26, 2018
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) type: enhancement A general enhancement
Projects
Archived in project
9 participants