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

Provide alternative for MD5 hashing in remember me token #8549

Closed
islamazhar opened this issue May 18, 2020 · 6 comments · Fixed by #11464
Closed

Provide alternative for MD5 hashing in remember me token #8549

islamazhar opened this issue May 18, 2020 · 6 comments · Fixed by #11464
Assignees
Labels
in: web An issue in web modules (web, webmvc) status: backported An issue that has been backported to maintenance branches status: ideal-for-contribution An issue that we actively are looking for someone to help us with type: enhancement A general enhancement
Milestone

Comments

@islamazhar
Copy link
Contributor

Expected Behavior

To hash passwords and secret keys a secure hashing algorithm (e.g., SHA256) should be used.

Current Behavior

The current Simple Hash-Based Token Approach uses MD5 hash digest for hashing password and secret-key in Remember-Me Authentication.

Corresponding Javadoc link and source code link

Context
MD5 is already proven to be a weak hashing algorithm and vulnerable against collision attacks [1] and modular differential attacks [2]. Hence I suggest using a secure hashing algorithm such as SHA-256 instead of already broken MD5 for remember me token.

References
[1] Den Boer and A. Bosselaers, “Collisions for the compression function of MD5,” in Workshop on the Theory and Application of CryptographicTechniques, pp. 293–304, Springer, 1993

[2] Wang, Xiaoyun, and Hongbo Yu. "How to break MD5 and other hash functions." Annual international conference on the theory and applications of cryptographic techniques. Springer, Berlin, Heidelberg, 2005.

@islamazhar islamazhar added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels May 18, 2020
@rwinch
Copy link
Member

rwinch commented May 21, 2020

Thanks for creating this issue.

In modern applications, I'd recommend using Spring Session's Remember Me support which allows you to easily offload the session into a data store without the need for cryptography.

I do agree that MD5 is not a good choice. Would you like to provide a pull request that provides the option to use SHA-256?

@rwinch rwinch added in: web An issue in web modules (web, webmvc) status: ideal-for-contribution An issue that we actively are looking for someone to help us with and removed status: waiting-for-triage An issue we've not yet triaged labels May 21, 2020
@aswinmahendiran
Copy link

Hello @islamazhar thanks for raising this issue.

Hello @rwinch I have raised the PR for the fix. I have updated the tests as well. Please check the PR (#8591).

This is my first PR in any Open Source project. Happy to start my journey in the security field I love the most!!

@aswinmahendiran
Copy link

Hi @rwinch it would be great if you can a look at the PR. I am excited to add my first contribution

@rwinch
Copy link
Member

rwinch commented May 28, 2020

@aswinmahendiran There is a duplicate PR #8580 (I have requested changes on it) Can you work with @islamazhar on which of you is sending a PR?

sargas added a commit to sargas/spring-security that referenced this issue Jan 31, 2021
A hashing algorithm property is added to TokenBasedRememberMeServices to
choose which algorithm is used when creating new Remember Me tokens.
This implementation is intended to preserve compatibility both with
Remember Me tokens that do not specify a hashing algorithm, and with
subclasses of TokenBasedRememberMeServices.

Closes spring-projectsgh-8549
@sargas
Copy link

sargas commented Feb 16, 2021

@islamazhar @aswinmahendiran not sure if either of you are still interested in this issue, but I have a PR at #9392 that supports SHA256 tokens without changing the existing behavior and allows for a future change of the default algorithm. I tried to update relevant parts of the documentation and tests accordingly.

marcusdacoregio pushed a commit to marcusdacoregio/spring-security that referenced this issue Jan 3, 2022
A hashing algorithm property is added to TokenBasedRememberMeServices to
choose which algorithm is used when creating new Remember Me tokens.
This implementation is intended to preserve compatibility both with
Remember Me tokens that do not specify a hashing algorithm, and with
subclasses of TokenBasedRememberMeServices.

Closes spring-projectsgh-8549
marcusdacoregio added a commit to marcusdacoregio/spring-security that referenced this issue Jan 4, 2022
@marcusdacoregio marcusdacoregio self-assigned this May 20, 2022
@rwinch rwinch added this to the 6.0.x milestone Jun 7, 2022
@rwinch
Copy link
Member

rwinch commented Jun 7, 2022

Given this is a breaking change we should consider it for 6.0.x or close it

@marcusdacoregio marcusdacoregio changed the title Replacing MD5 hashing for remember me token Provide alternative for MD5 hashing in remember me token Jun 30, 2022
@marcusdacoregio marcusdacoregio modified the milestones: 6.0.x, 5.8.0-M1 Jul 13, 2022
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: backported An issue that has been backported to maintenance branches status: ideal-for-contribution An issue that we actively are looking for someone to help us with type: enhancement A general enhancement
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants