diff --git a/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java b/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java index 21efe009462..c59246320d6 100644 --- a/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java +++ b/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java @@ -20,6 +20,7 @@ import org.springframework.security.crypto.password.PasswordEncoder; import java.security.SecureRandom; +import java.util.regex.Matcher; import java.util.regex.Pattern; /** @@ -32,7 +33,7 @@ */ public class BCryptPasswordEncoder implements PasswordEncoder { private Pattern BCRYPT_PATTERN = Pattern - .compile("\\A\\$2(a|y|b)?\\$\\d\\d\\$[./0-9A-Za-z]{53}"); + .compile("\\A\\$2(a|y|b)?\\$(\\d\\d)\\$[./0-9A-Za-z]{53}"); private final Log logger = LogFactory.getLog(getClass()); private final int strength; @@ -93,20 +94,16 @@ public BCryptPasswordEncoder(BCryptVersion version, int strength, SecureRandom r throw new IllegalArgumentException("Bad strength"); } this.version = version; - this.strength = strength; + this.strength = strength == -1 ? 10 : strength; this.random = random; } public String encode(CharSequence rawPassword) { String salt; - if (strength > 0) { - if (random != null) { - salt = BCrypt.gensalt(version.getVersion(), strength, random); - } else { - salt = BCrypt.gensalt(version.getVersion(), strength); - } + if (random != null) { + salt = BCrypt.gensalt(version.getVersion(), strength, random); } else { - salt = BCrypt.gensalt(version.getVersion()); + salt = BCrypt.gensalt(version.getVersion(), strength); } return BCrypt.hashpw(rawPassword.toString(), salt); } @@ -125,6 +122,23 @@ public boolean matches(CharSequence rawPassword, String encodedPassword) { return BCrypt.checkpw(rawPassword.toString(), encodedPassword); } + @Override + public boolean upgradeEncoding(String encodedPassword) { + if (encodedPassword == null || encodedPassword.length() == 0) { + logger.warn("Empty encoded password"); + return false; + } + + Matcher matcher = BCRYPT_PATTERN.matcher(encodedPassword); + if (!matcher.matches()) { + throw new IllegalArgumentException("Encoded password does not look like BCrypt: " + encodedPassword); + } + else { + int strength = Integer.parseInt(matcher.group(2)); + return strength < this.strength; + } + } + /** * Stores the default bcrypt version for use in configuration. * diff --git a/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java b/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java index c9bd476c3c9..4bc4e68083a 100644 --- a/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java +++ b/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java @@ -217,9 +217,15 @@ private String extractId(String prefixEncodedPassword) { } @Override - public boolean upgradeEncoding(String encodedPassword) { - String id = extractId(encodedPassword); - return !this.idForEncode.equalsIgnoreCase(id); + public boolean upgradeEncoding(String prefixEncodedPassword) { + String id = extractId(prefixEncodedPassword); + if (!this.idForEncode.equalsIgnoreCase(id)) { + return true; + } + else { + String encodedPassword = extractEncodedPassword(prefixEncodedPassword); + return this.idToPasswordEncoder.get(id).upgradeEncoding(encodedPassword); + } } private String extractEncodedPassword(String prefixEncodedPassword) { diff --git a/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java b/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java index 9f574d6c51e..28ac723bce6 100644 --- a/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java +++ b/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java @@ -169,4 +169,35 @@ public void doesntMatchBogusEncodedValue() { assertThat(encoder.matches("password", "012345678901234567890123456789")).isFalse(); } + @Test + public void upgradeFromLowerStrength() { + BCryptPasswordEncoder weakEncoder = new BCryptPasswordEncoder(5); + BCryptPasswordEncoder strongEncoder = new BCryptPasswordEncoder(15); + + String weakPassword = weakEncoder.encode("password"); + String strongPassword = strongEncoder.encode("password"); + + assertThat(weakEncoder.upgradeEncoding(strongPassword)).isFalse(); + assertThat(strongEncoder.upgradeEncoding(weakPassword)).isTrue(); + } + + /** + * @see https://github.com/spring-projects/spring-security/pull/7042#issuecomment-506755496 + */ + @Test + public void upgradeFromNullOrEmpty() { + BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(); + assertThat(encoder.upgradeEncoding(null)).isFalse(); + assertThat(encoder.upgradeEncoding("")).isFalse(); + } + + /** + * @see https://github.com/spring-projects/spring-security/pull/7042#issuecomment-506755496 + */ + @Test(expected = IllegalArgumentException.class) + public void upgradeFromNonBCrypt() { + BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(); + encoder.upgradeEncoding("not-a-bcrypt-password"); + } + } diff --git a/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java b/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java index cd24afca479..6fdea55a840 100644 --- a/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java +++ b/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java @@ -215,12 +215,16 @@ public void upgradeEncodingWhenIdInvalidFormatThenTrue() { } @Test - public void upgradeEncodingWhenSameIdThenFalse() { - assertThat(this.passwordEncoder.upgradeEncoding(this.bcryptEncodedPassword)).isFalse(); + public void upgradeEncodingWhenSameIdThenEncoderDecides() { + this.passwordEncoder.upgradeEncoding(this.bcryptEncodedPassword); + + verify(bcrypt).upgradeEncoding(this.encodedPassword); } @Test public void upgradeEncodingWhenDifferentIdThenTrue() { assertThat(this.passwordEncoder.upgradeEncoding(this.noopEncodedPassword)).isTrue(); + + verifyZeroInteractions(bcrypt); } }