Spring-boot: Add Support for Password Encoding to Spring CLI

[rwinch]

Spring Boot provides the ability to externalize a user's password using Spring Environment. However, the user does not have an easy way to encode the password to keep it secure.

It would be nice if the Spring CLI provided a command for encoding a password. This would require a dependency on spring-security-crypto which only has a dependency on spring-jcl and an optional dependency on org.bouncycastle:bcpkix-jdk15on

[spencergibb]

The spring cloud cli provides similar functionality, BTW.

[philwebb]

Myself and Rob had a quick call about this. Although we've been trying to de-emphasize the Spring CLI I think this one does actually make sense there.

The options we discussed were:

• An online tool - sucks because you're sending passwords out
• A main method in the spring-crypto jar - sucks because it will mess with main method detection in Boot, and you need to download the jar somehow
• A dedicated tool released by Spring Security - sucks because there's one more brew/install dance for the user
• A CLI extension - doesn't suck too bad, but Spring Security need to release another JAR and the user needs to do $ spring install org.springframework.security:spring-security-cli:1.2.3.RELEASE

Since Boot supports password in properties, having a tool shipped that allows you to generate them makes sense to me.

[philwebb]

Flagging for team attention in case anyone feels strongly that adding to the CLI would be an issue.

[wilkinsona]

FWIW, I think this makes sense too.

[philwebb]

@rwinch I've added support for this. The only thing I'm not sure about is should we include the {alg} prefix in the result. Currently it does not.

You can do spring encodepassword mysecret and it'll spit out the encoded version. You can also do spring encodepassword -a pbkdf2 mysecret if you want to use a different algorithm. Currently only pbkdf2 and bcrypt seem sensible. The others are either deprecated or need bouncy castle.

[rwinch]

@philwebb Thanks for the fast turnaround. I think that it makes sense to support PasswordEncoderFactories.createDelegatingPasswordEncoeder() and make that the default. This will ensure that {alg} is added automatically and the default encoding matches Spring Security's default. Other than that I agree and think that bcrypt and pbkdf2 make sense as the other algorithms we should support. Thoughts?

[spencergibb]

It would be nice if it were compatible with https://github.com/spring-cloud/spring-cloud-cli/tree/master/spring-cloud-cli/src/main/java/org/springframework/cloud/cli/command/encrypt

Then we could remove that.

[philwebb]

@rwinch I looked at PasswordEncoderFactories but I don't think I can really use it. The DelegatingPasswordEncoder.encode method always uses the default so there'd be no way to specify the algorithm.

[philwebb]

@spencergibb Aren't they for different things? This is for one-way password hash generation. Also the prefix thing wouldn't really work either would it?

[spencergibb]

@philwebb thanks yes, they are different sorry for the misunderstanding.

[rwinch]

@philwebb

I looked at PasswordEncoderFactories but I don't think I can really use it. The DelegatingPasswordEncoder.encode method always uses the default so there'd be no way to specify the algorithm.

I'm not sure I understand the problem.

Right now EncodePasswordCommand delegates to either BCryptPasswordEncoder or Pbkdf2PasswordEncoder. Could it be updated to have a default that is used if no algorithm is used. For example:

encoders.put("bcrypt", BCryptPasswordEncoder::new);
encoders.put("default", PasswordEncoderFactories::createDelegatingPasswordEncoeder);
encoders.put("pbkdf2", Pbkdf2PasswordEncoder::new);

Then you could change the default value of the algorithm option to be default.

[philwebb]

Oh I see, yeah we could do that. I thought you mean pull the same names out of the map which we can't do.

[philwebb]

Actually, thinking some more I don't think it's such a good idea. The default one won't add the prefix and we probably should always add the prefix to protect against any future default algorithm change. In other words, if you encode a passoword for a properties, you don't want some future upgrade to suddenly start checking it with a different algorithm.

[rwinch]

I think there is some confusion.

The default one won't add the prefix and we probably should always add the prefix to protect against any future default algorithm change.

DelegatingPasswordEncoder.encode does automatically add the prefix for the encoder that is being used to encode. This is because the encode method is intended to be used along with its matches.

In other words, if you encode a passoword for a properties, you don't want some future upgrade to suddenly start checking it with a different algorithm.

The prefix is not a standard, so it will not work with other libraries. Users might be encoding the password with Spring Security and validating the password with a Ruby application. It is perfectly reasonable for users to explicitly provide BCryptPasswordEncoder or Pbkdf2PasswordEncoder.

For these reasons, when a user specifies the algorithm as bcrypt or pbkdf2 we should not prefix the encoded password in the output. If we do that, we are really encoding it with DelegatingPasswordEncoder + alg which will not work with other libraries. Adding the prefix when the alg is provided will likely surprise users who do not know what the format of the alg they chose is.

It is important to remember that migration to using the prefix is as simple as concatenating the prefix with the hashed password. Since migration is simple, for new applications it is likely appealing to stick with a standard encoding (avoid using DelegatingPasswordEncoder) until it is necessary. This is yet another reason we should avoid prefixing the password when the alg is provided by the user.

[philwebb]

Ahh my mistake. For some reason I thought that DelegatingPasswordEncoder.encode didn't add the prefix.