Spring-security: Legacy Migration Incompatibility - Why does MessageDigestPasswordEncoder consider '{}' to be part of the salt?

[phaninra]

Summary

I'm upgrading from Spring Security 4.x to 5.x.

The ReflectionSaltSource from Spring 4 lets us configure a custom salt. But that's removed in Spring Security 5. I then found out that I should use MessageDigestPasswordEncoder. It has a detailed java-doc but unfortunately the doc is like a bag of words without conveying any structured information (I tried multiple times; my bad if I was ignorant).

Anyways I thought I should do the following based on my limited understanding of the javadoc.

Old system with 4.x - myEncodedPassword and mySalt are passed separately to the encoder.

New System with 5.x - Pass one field with the value {mySalt}myEncodedPassword to the MessageDigestPasswordEncoder

Actual Behavior

The Problem was that when MessageDigestPasswordEncoder sees {mySalt}encodedPassword, it uses {mySalt} (with the {}) as the salt instead of using mySalt as the salt . I'm confused.

Expected Behavior

I'd expect the salt to be mySalt and not {mySalt}

Version

group='org.springframework.security', module='spring-security-core', version='5.1.4.RELEASE'

Sample

I've used Groovy to create a concise sample.

@Grab(group='org.springframework.security', module='spring-security-core', version='5.1.4.RELEASE')
import org.springframework.security.crypto.password.MessageDigestPasswordEncoder

String password = 'myPassword'
String salt_1 = 'mySalt'
String salt_2 = '{mySalt}'
// http://www.lorem-ipsum.co.uk/hasher.php generated below hashes for above salt and password combinations
String encodedPasswordWithSalt_1 = '57bc828628811a10496215e217b7ae9b714c859fc7a8b1c678c9a0cc40aac422'
String encodedPasswordWithSalt_2 = 'a18b53fc58843def1e08e00a718f40d6f8eda0b97ef97824b5078c1fad93c0c5'

MessageDigestPasswordEncoder encoder = new MessageDigestPasswordEncoder('SHA-256')

// expected to match but did not
println "expected=true, actual=" + encoder.matches(password, "{${salt_1}}${encodedPasswordWithSalt_1}") 

// why does this match?
println "expected=false, actual=" + encoder.matches(password, "{${salt_1}}${encodedPasswordWithSalt_2}") 

[clevertension]

MessageDigestPasswordEncoder is deprecated in 5.x, so you can implement your own PasswordEncoder, it is extensible

public class YourPasswordEncoder implements PasswordEncoder {
}

[phaninra]

Thanks for the response. We have migrated to scrypt but a number old accounts are still tied to sha256. So we need to continue supporting sha256.

Q1. Is a custom extension the only solution to implement sha256 in Spring Security 5.x?

Q2. Is it a bug in MessageDigestPasswordEncoder to consume {} as part of the salt? If I understand correctly, the javadoc describes a different behavior (without the {}).

Also the MessageDigestPasswordEncoder is not a traditionally deprecated class (for removal). It's just deprecated as an alarm.

[akhmelov]

@clevertension

There are no plans to remove this support. It is deprecated to indicate that this is a legacy implementation and using it is considered insecure.

[abakran1]

I came across the same issue where I see the {} are being used as part of the salt in MessageDigestPasswordEncoder. Why offer the legacy migration process if it is not supported at all, we need a way for legacy users to migrate over.