Derive hash length for Pbkdf2PasswordEncoder

crypto/src/main/java/org/springframework/security/crypto/argon2/Argon2PasswordEncoder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -52,9 +52,9 @@ public class Argon2PasswordEncoder implements PasswordEncoder {
 
 	private static final int DEFAULT_PARALLELISM = 1;
 
-	private static final int DEFAULT_MEMORY = 1 << 12;
+	private static final int DEFAULT_MEMORY = 1 << 14;
 
-	private static final int DEFAULT_ITERATIONS = 3;
+	private static final int DEFAULT_ITERATIONS = 2;
 
 	private final Log logger = LogFactory.getLog(getClass());
 

crypto/src/main/java/org/springframework/security/crypto/factory/PasswordEncoderFactories.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -71,7 +71,15 @@ public static PasswordEncoder createDelegatingPasswordEncoder() {
 		encoders.put("MD4", new org.springframework.security.crypto.password.Md4PasswordEncoder());
 		encoders.put("MD5", new org.springframework.security.crypto.password.MessageDigestPasswordEncoder("MD5"));
 		encoders.put("noop", org.springframework.security.crypto.password.NoOpPasswordEncoder.getInstance());
-		encoders.put("pbkdf2", new Pbkdf2PasswordEncoder());
+
+		// Preserve the original settings for PBKDF2 for backwards compatibility.
+		// The default configuration has been updated to the recommended minimums.
+		// See gh-10506 Update PasswordEncoder Minimums
+		Pbkdf2PasswordEncoder pbkdf2PasswordEncoder = new Pbkdf2PasswordEncoder("", 8, 185000, 256);
+		pbkdf2PasswordEncoder.setAlgorithm(Pbkdf2PasswordEncoder.SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA1);
+		encoders.put("pbkdf2", pbkdf2PasswordEncoder);
+
+		encoders.put("pbkdf2-sha256", new Pbkdf2PasswordEncoder());
 		encoders.put("scrypt", new SCryptPasswordEncoder());
 		encoders.put("SHA-1", new org.springframework.security.crypto.password.MessageDigestPasswordEncoder("SHA-1"));
 		encoders.put("SHA-256",

crypto/src/main/java/org/springframework/security/crypto/password/Pbkdf2PasswordEncoder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,8 +36,6 @@
  * <li>a configurable random salt value length (default is {@value #DEFAULT_SALT_LENGTH}
  * bytes)</li>
  * <li>a configurable number of iterations (default is {@value #DEFAULT_ITERATIONS})</li>
- * <li>a configurable output hash width (default is {@value #DEFAULT_HASH_WIDTH}
- * bits)</li>
  * <li>a configurable key derivation function (see {@link SecretKeyFactoryAlgorithm})</li>
  * <li>a configurable secret appended to the random salt (default is empty)</li>
  * </ul>
@@ -50,30 +48,45 @@
  */
 public class Pbkdf2PasswordEncoder implements PasswordEncoder {
 
-	private static final int DEFAULT_SALT_LENGTH = 8;
+	private static final int DEFAULT_SALT_LENGTH = 16;
 
-	private static final int DEFAULT_HASH_WIDTH = 256;
-
-	private static final int DEFAULT_ITERATIONS = 185000;
+	private static final int DEFAULT_ITERATIONS = 310000;
 
 	private final BytesKeyGenerator saltGenerator;
 
 	private final byte[] secret;
 
-	private final int hashWidth;
-
 	private final int iterations;
 
-	private String algorithm = SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA1.name();
+	private String algorithm = SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA256.name();
+
+	private int hashWidth = 256; // SHA-256
+
+	// @formatter:off
+	/*
+	The length of the hash should be derived from the hashing algorithm.
+
+	For example:
+		SHA-1 - 160 bits (20 bytes)
+		SHA-256 - 256 bits (32 bytes)
+		SHA-512 - 512 bits (64 bytes)
+
+	However, the original configuration for PBKDF2 was hashWidth=256 and algorithm=SHA-1, which is incorrect.
+	The default configuration has been updated to hashWidth=256 and algorithm=SHA-256 (see gh-10506).
+	In order to preserve backwards compatibility, the variable 'overrideHashWidth' has been introduced
+	to indicate usage of the deprecated constructors that allow (and honor) a hashWidth parameter.
+	 */
+	// @formatter:on
+	private boolean overrideHashWidth = true;
 
 	private boolean encodeHashAsBase64;
 
 	/**
 	 * Constructs a PBKDF2 password encoder with no additional secret value. There will be
 	 * a salt length of {@value #DEFAULT_SALT_LENGTH} bytes, {@value #DEFAULT_ITERATIONS}
-	 * iterations and a hash width of {@value #DEFAULT_HASH_WIDTH} bits. The default is
-	 * based upon aiming for .5 seconds to validate the password when this class was
-	 * added. Users should tune password verification to their own systems.
+	 * iterations and SHA-256 algorithm. The default is based upon aiming for .5 seconds
+	 * to validate the password when this class was added. Users should tune password
+	 * verification to their own systems.
 	 */
 	public Pbkdf2PasswordEncoder() {
 		this("");
@@ -82,24 +95,22 @@ public Pbkdf2PasswordEncoder() {
 	/**
 	 * Constructs a standard password encoder with a secret value which is also included
 	 * in the password hash. There will be a salt length of {@value #DEFAULT_SALT_LENGTH}
-	 * bytes, {@value #DEFAULT_ITERATIONS} iterations and a hash width of
-	 * {@value #DEFAULT_HASH_WIDTH} bits.
+	 * bytes, {@value #DEFAULT_ITERATIONS} iterations and SHA-256 algorithm.
 	 * @param secret the secret key used in the encoding process (should not be shared)
 	 */
 	public Pbkdf2PasswordEncoder(CharSequence secret) {
-		this(secret, DEFAULT_SALT_LENGTH, DEFAULT_ITERATIONS, DEFAULT_HASH_WIDTH);
+		this(secret, DEFAULT_SALT_LENGTH);
 	}
 
 	/**
 	 * Constructs a standard password encoder with a secret value as well as salt length.
-	 * There will be {@value #DEFAULT_ITERATIONS} iterations and a hash width of
-	 * {@value #DEFAULT_HASH_WIDTH} bits.
+	 * There will be {@value #DEFAULT_ITERATIONS} iterations and SHA-256 algorithm.
 	 * @param secret the secret
 	 * @param saltLength the salt length (in bytes)
 	 * @since 5.5
 	 */
 	public Pbkdf2PasswordEncoder(CharSequence secret, int saltLength) {
-		this(secret, saltLength, DEFAULT_ITERATIONS, DEFAULT_HASH_WIDTH);
+		this(secret, saltLength, DEFAULT_ITERATIONS, SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA256);
 	}
 
 	/**
@@ -109,7 +120,11 @@ public Pbkdf2PasswordEncoder(CharSequence secret, int saltLength) {
 	 * @param iterations the number of iterations. Users should aim for taking about .5
 	 * seconds on their own system.
 	 * @param hashWidth the size of the hash (in bits)
+	 * @deprecated Use
+	 * {@link #Pbkdf2PasswordEncoder(CharSequence, int, int, SecretKeyFactoryAlgorithm)}
+	 * instead
 	 */
+	@Deprecated
 	public Pbkdf2PasswordEncoder(CharSequence secret, int iterations, int hashWidth) {
 		this(secret, DEFAULT_SALT_LENGTH, iterations, hashWidth);
 	}
@@ -123,12 +138,36 @@ public Pbkdf2PasswordEncoder(CharSequence secret, int iterations, int hashWidth)
 	 * seconds on their own system.
 	 * @param hashWidth the size of the hash (in bits)
 	 * @since 5.5
+	 * @deprecated Use
+	 * {@link #Pbkdf2PasswordEncoder(CharSequence, int, int, SecretKeyFactoryAlgorithm)}
+	 * instead
 	 */
+	@Deprecated
 	public Pbkdf2PasswordEncoder(CharSequence secret, int saltLength, int iterations, int hashWidth) {
 		this.secret = Utf8.encode(secret);
 		this.saltGenerator = KeyGenerators.secureRandom(saltLength);
 		this.iterations = iterations;
 		this.hashWidth = hashWidth;
+		this.overrideHashWidth = false; // Honor 'hashWidth' to preserve backwards
+										// compatibility
+	}
+
+	/**
+	 * Constructs a standard password encoder with a secret value as well as salt length,
+	 * iterations and algorithm.
+	 * @param secret the secret
+	 * @param saltLength the salt length (in bytes)
+	 * @param iterations the number of iterations. Users should aim for taking about .5
+	 * seconds on their own system.
+	 * @param secretKeyFactoryAlgorithm the algorithm to use
+	 * @since 6.0
+	 */
+	public Pbkdf2PasswordEncoder(CharSequence secret, int saltLength, int iterations,
+			SecretKeyFactoryAlgorithm secretKeyFactoryAlgorithm) {
+		this.secret = Utf8.encode(secret);
+		this.saltGenerator = KeyGenerators.secureRandom(saltLength);
+		this.iterations = iterations;
+		setAlgorithm(secretKeyFactoryAlgorithm);
 	}
 
 	/**
@@ -153,6 +192,10 @@ public void setAlgorithm(SecretKeyFactoryAlgorithm secretKeyFactoryAlgorithm) {
 		catch (NoSuchAlgorithmException ex) {
 			throw new IllegalArgumentException("Invalid algorithm '" + algorithmName + "'.", ex);
 		}
+		if (this.overrideHashWidth) {
+			this.hashWidth = SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA1.equals(secretKeyFactoryAlgorithm) ? 160
+					: SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA256.equals(secretKeyFactoryAlgorithm) ? 256 : 512;
+		}
 	}
 
 	/**

crypto/src/main/java/org/springframework/security/crypto/scrypt/SCryptPasswordEncoder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -58,6 +58,16 @@
  */
 public class SCryptPasswordEncoder implements PasswordEncoder {
 
+	private static final int DEFAULT_CPU_COST = 65536;
+
+	private static final int DEFAULT_MEMORY_COST = 8;
+
+	private static final int DEFAULT_PARALLELISM = 1;
+
+	private static final int DEFAULT_KEY_LENGTH = 32;
+
+	private static final int DEFAULT_SALT_LENGTH = 16;
+
 	private final Log logger = LogFactory.getLog(getClass());
 
 	private final int cpuCost;
@@ -71,7 +81,7 @@ public class SCryptPasswordEncoder implements PasswordEncoder {
 	private final BytesKeyGenerator saltGenerator;
 
 	public SCryptPasswordEncoder() {
-		this(16384, 8, 1, 32, 64);
+		this(DEFAULT_CPU_COST, DEFAULT_MEMORY_COST, DEFAULT_PARALLELISM, DEFAULT_KEY_LENGTH, DEFAULT_SALT_LENGTH);
 	}
 
 	/**

crypto/src/test/java/org/springframework/security/crypto/argon2/Argon2PasswordEncoderTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,7 +39,10 @@ public class Argon2PasswordEncoderTests {
 	@Mock
 	private BytesKeyGenerator keyGeneratorMock;
 
-	private Argon2PasswordEncoder encoder = new Argon2PasswordEncoder();
+	// Restore the original settings for Argon2.
+	// The default configuration has been updated to the recommended minimums.
+	// See gh-10506 Update PasswordEncoder Minimums
+	private Argon2PasswordEncoder encoder = new Argon2PasswordEncoder(16, 32, 1, 1 << 12, 3);
 
 	@Test
 	public void encodeDoesNotEqualPassword() {
@@ -127,6 +130,15 @@ public void encodeWhenUsingPredictableSaltWithCustomParamsThenEqualTestHash() th
 				"$argon2id$v=19$m=512,t=5,p=4$QUFBQUFBQUFBQUFBQUFBQQ$PNv4C3K50bz3rmON+LtFpdisD7ePieLNq+l5iUHgc1k");
 	}
 
+	@Test
+	public void encodeWhenUsingPredictableSaltWithDefaultParamsThenEqualTestHash() throws Exception {
+		this.encoder = new Argon2PasswordEncoder();
+		injectPredictableSaltGen();
+		String hash = this.encoder.encode("sometestpassword");
+		assertThat(hash).isEqualTo(
+				"$argon2id$v=19$m=16384,t=2,p=1$QUFBQUFBQUFBQUFBQUFBQQ$zGt5MiNPSUOo4/7jBcJMayCPfcsLJ4c0WUxhwGDIYPw");
+	}
+
 	@Test
 	public void upgradeEncodingWhenSameEncodingThenFalse() {
 		String hash = this.encoder.encode("password");
@@ -135,7 +147,7 @@ public void upgradeEncodingWhenSameEncodingThenFalse() {
 
 	@Test
 	public void upgradeEncodingWhenSameStandardParamsThenFalse() {
-		Argon2PasswordEncoder newEncoder = new Argon2PasswordEncoder();
+		Argon2PasswordEncoder newEncoder = new Argon2PasswordEncoder(16, 32, 1, 1 << 12, 3);
 		String hash = this.encoder.encode("password");
 		assertThat(newEncoder.upgradeEncoding(hash)).isFalse();
 	}

crypto/src/test/java/org/springframework/security/crypto/factory/PasswordEncoderFactoriesTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -75,6 +75,12 @@ public void matchesWhenPbkdf2ThenWorks() {
 		assertThat(this.encoder.matches(this.rawPassword, encodedPassword)).isTrue();
 	}
 
+	@Test
+	public void matchesWhenPbkdf2Sha256ThenWorks() {
+		String encodedPassword = "{pbkdf2-sha256}fefe5120467e5d4ccff442dbb2fa86d276262d97435c0c54e5eebced51ffd144fcb05eb53fea2677216c4f3250010006";
+		assertThat(this.encoder.matches(this.rawPassword, encodedPassword)).isTrue();
+	}
+
 	@Test
 	public void matchesWhenSCryptThenWorks() {
 		String encodedPassword = "{scrypt}$e0801$8bWJaSu2IKSn9Z9kM+TPXfOc/9bdYSrN1oD9qfVThWEwdRTnO7re7Ei+fUZRJ68k9lTyuTeUp4of4g24hHnazw==$OAOec05+bXxvuu/1qZ6NUR+xQYvYv7BeL1QxwRpY5Pc=";

crypto/src/test/java/org/springframework/security/crypto/password/Pbkdf2PasswordEncoderTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
 
 import java.util.Arrays;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.security.crypto.codec.Hex;
@@ -28,11 +29,23 @@
 
 public class Pbkdf2PasswordEncoderTests {
 
-	private Pbkdf2PasswordEncoder encoder = new Pbkdf2PasswordEncoder("secret");
+	private Pbkdf2PasswordEncoder encoder;
 
-	private Pbkdf2PasswordEncoder encoderSalt16 = new Pbkdf2PasswordEncoder("", 16);
+	private Pbkdf2PasswordEncoder encoderSalt16;
 
-	private Pbkdf2PasswordEncoder[] encoders = new Pbkdf2PasswordEncoder[] { this.encoder, this.encoderSalt16 };
+	private Pbkdf2PasswordEncoder[] encoders;
+
+	@BeforeEach
+	public void setup() {
+		// Restore the original settings for PBKDF2 for backwards compatibility.
+		// The default configuration has been updated to the recommended minimums.
+		// See gh-10506 Update PasswordEncoder Minimums
+		this.encoder = new Pbkdf2PasswordEncoder("secret", 8, 185000, 256);
+		this.encoder.setAlgorithm(Pbkdf2PasswordEncoder.SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA1);
+		this.encoderSalt16 = new Pbkdf2PasswordEncoder("", 16, 185000, 256);
+		this.encoderSalt16.setAlgorithm(Pbkdf2PasswordEncoder.SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA1);
+		this.encoders = new Pbkdf2PasswordEncoder[] { this.encoder, this.encoderSalt16 };
+	}
 
 	@Test
 	public void encodedLengthSuccess() {
@@ -50,6 +63,14 @@ public void matches() {
 		assertThat(this.encoder.matches("password", result)).isTrue();
 	}
 
+	@Test
+	public void matchesWhenDefaultsThenSuccess() {
+		Pbkdf2PasswordEncoder encoder = new Pbkdf2PasswordEncoder();
+		String rawPassword = "password";
+		String encodedPassword = "fefe5120467e5d4ccff442dbb2fa86d276262d97435c0c54e5eebced51ffd144fcb05eb53fea2677216c4f3250010006";
+		assertThat(encoder.matches(rawPassword, encodedPassword)).isTrue();
+	}
+
 	@Test
 	public void matchesWhenCustomSaltLengthThenSuccess() {
 		String result = this.encoderSalt16.encode("password");