Skip to content

Add DelegatingPasswordEncoder #4666

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
rwinch opened this issue Oct 20, 2017 · 3 comments
Closed

Add DelegatingPasswordEncoder #4666

rwinch opened this issue Oct 20, 2017 · 3 comments
Assignees
Labels
in: crypto An issue in spring-security-crypto type: enhancement A general enhancement
Milestone

Comments

@rwinch
Copy link
Member

rwinch commented Oct 20, 2017

Summary

Create a DelegatingPasswordEncoder which supports delegating to multiple different PasswordEncoder implementations.

String idForEncode = "bcrypt";
Map<String,PasswordEncoder> encoders = new HashMap<>();
encoders.put(idForEncode, new BCryptPasswordEncoder());
encoders.put("noop", NoOpPasswordEncoder.getInstance());
encoders.put("pbkdf2", new Pbkdf2PasswordEncoder());
encoders.put("scrypt", new SCryptPasswordEncoder());
encoders.put("sha256", new StandardPasswordEncoder());

PasswordEncoder passwordEncoder = new DelegatingPasswordEncoder(idForEncode, encoders);

The following values would be stored:

{bcrypt}$2a$10$dXJ3SW6G7P50lGmMkkmwe.20cQQubK3.HZWzG3YB1tlRy.fqvM/BG
{noop}password
{pbkdf2}5d923b44a6d129f3ddf3e3c8d29412723dcbde72445e8ef6bf3b508fbf17fa4ed4d6b99ca763d8dc
{scrypt}$e0801$8bWJaSu2IKSn9Z9kM+TPXfOc/9bdYSrN1oD9qfVThWEwdRTnO7re7Ei+fUZRJ68k9lTyuTeUp4of4g24hHnazw==$OAOec05+bXxvuu/1qZ6NUR+xQYvYv7BeL1QxwRpY5Pc=
{sha256}97cde38028ad898ebc02e690819fa220e88c62e0699403e94fff291cfffaf8410849f27605abcbc0

We would then use the following logic for matching:

  • The first password would have a PasswordEncoder id of bcrypt and
    encodedPassword of $2a$10$dXJ3SW6G7P50lGmMkkmwe.20cQQubK3.HZWzG3YB1tlRy.fqvM/BG.
    When matching it would delegate to
    BCryptPasswordEncoder
  • The second password would have a PasswordEncoder id of noop and
    encodedPassword of password. When matching it would delegate to NoOpPasswordEncoder
  • The third password would have a PasswordEncoder id of pbkdf2 and
    encodedPassword of
    5d923b44a6d129f3ddf3e3c8d29412723dcbde72445e8ef6bf3b508fbf17fa4ed4d6b99ca763d8dc.
    When matching it would delegate to Pbkdf2PasswordEncoder
  • The fourth password would have a PasswordEncoder id of scrypt and
    encodedPassword of
    $e0801$8bWJaSu2IKSn9Z9kM+TPXfOc/9bdYSrN1oD9qfVThWEwdRTnO7re7Ei+fUZRJ68k9lTyuTeUp4of4g24hHnazw==$OAOec05+bXxvuu/1qZ6NUR+xQYvYv7BeL1QxwRpY5Pc=
    When matching it would delegate to
    SCryptPasswordEncoder
  • The final password would have a PasswordEncoder id of sha256 and
    encodedPassword of
    97cde38028ad898ebc02e690819fa220e88c62e0699403e94fff291cfffaf8410849f27605abcbc0.
    When matching it would delegate to StandardPasswordEncoder
@rwinch rwinch added in: crypto An issue in spring-security-crypto type: enhancement A general enhancement labels Oct 20, 2017
@rwinch rwinch added this to the 5.0.0.RC1 milestone Oct 20, 2017
@rwinch rwinch self-assigned this Oct 20, 2017
@rwinch rwinch closed this as completed in d0332eb Oct 20, 2017
rwinch added a commit that referenced this issue Oct 20, 2017
@rajjaiswalsaumya
Copy link

Most security systems aims at even administrator should not be able to make out what passwords are. I think storing plain {pbkdf2} or {bcrypt} or any such keys along with passwords just to make out what encryption strategy is used may pose a security threat. It would be great if the encryption of such keys stored along with passwords is also encrypted via a configurable strategy in the application. So that people might configure which encryption strategy is to be used to encrypt encryption keys. I store password started in amazon S3 bucket, which is connected at time of application boot. This is one way of implementing the password startegy which even my db administrator is not even aware of. You might think of many such usecases.

In the nutshell, even plain passwords to db administrator should be hard to decrypt.

@rwinch
Copy link
Member Author

rwinch commented Oct 22, 2017

I think storing plain {pbkdf2} or {bcrypt} or any such keys along with passwords just to make out what

These are not keys. They are identifiers which is a lot different. Key's must be kept secret, but ids do not need to be.

encryption strategy is used may pose a security threat.

First we are not encrypting the passwords. I think this might be where the disconnect is. This mechanism is only for validating a user's password. It is not used for storing a password that must be used presented later on (i.e. for connecting to a database). The storage mechanisms for these two use cases are quite different. You do not encrypt passwords for users. You hash them using a one way hash.

Second, it only takes basic knowledge of hashing algorithms to discover which hashing algorithm was used. For example, if passwords start with $2a$ it is very likely using BCrypt as this is the version identifier for BCrypt. Another big clue is which characters are being outputted and the number of bytes. In most instances a human can guess what hashing algorithm a password is stored in, but for a computer it is best to store the format so there is no room for ambiguity.

Finally, the fundamental principals of cryptography state that it should work even if the adversary knows the algorithm you are using. So, giving this information away, should not be a security thread.

Can you provide any evidence that this is a security thread? This mechanism was the recommendation of password experts as discussed on #2774 so I'd be very interested if you have some additional insights.

I will add that you can use whatever identifier you want to map it to another PasswordEncoder so they don't have to be as obvious if you really feel like this is an advantage. However, in practice this will not buy you anything other than a more difficult time for developers. If you are not wanting to support different password storage mechanisms and password upgrades (as was done in Security 4.x) you can always revert to the old mechanism by providing a different PasswordEncoder mechanism (i.e. BCryptPasswordEncoder)

@rajjaiswalsaumya
Copy link

@rwinch Rob, with time, the data of rainbow table has grown too large and surprisingly even a salted hash can also be broken. To quote, I was surprised my password hash whose password was 10 character long with special symbol, small and uppercase and also with digits, was broken by rainbow table attack in few minutes.

Apart from that
If the password hash and salt is known to someone, he will have to guess the algo, in order to find the hash (which of course are several). But if the algo is known, then its easy for him to do rainbow table attack because he just needs to find correct table and use its hashes to get the password.

I am not sure about security experts, but in our security company its not allowed to expose what algo we are using for cryptography. Only us and our S3 bucket knows what it is. Our legacy systems after user creates a password, keeps on changing the hashes after every login by changing salts. User is bound to change his password after certain logins, to ensure his credentials privacy.

The password algo and key strength is changed after every deployment and the passwords are migrated once user logs in with updated timestamp.

I like the idea of keeping the algo as part of key, anyway we also need to keep a track of it in application with datetime of deployment, to migrate user passwords. All I was saying if that algo name could be encrypted becomes beneficial for secured critical systems.

thomasdarimont pushed a commit to thomasdarimont/spring-security that referenced this issue Apr 25, 2018
thomasdarimont pushed a commit to thomasdarimont/spring-security that referenced this issue Apr 25, 2018
thomasdarimont pushed a commit to thomasdarimont/spring-security that referenced this issue Apr 25, 2018
thomasdarimont pushed a commit to thomasdarimont/spring-security that referenced this issue Apr 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: crypto An issue in spring-security-crypto type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants