Make salt length configurable in Pbkdf2PasswordEncoder

Add constructors with a salt length input parameter.
Default salt length is still 8-byte long like before when
saltGenerator was initialized with call to
KeyGenerators#secureRandom() which use
SecureRandomBytesKeyGenerator#DEFAULT_KEY_LENGTH.

Closes gh-4372

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -31,11 +31,16 @@ import org.springframework.security.crypto.keygen.KeyGenerators;
 import org.springframework.security.crypto.util.EncodingUtils;
 
 /**
- * A {@code PasswordEncoder} implementation that uses PBKDF2 with a configurable number of
- * iterations and a random 8-byte random salt value.
- * <p>
- * The width of the output hash can also be configured.
- * <p>
+ * A {@link PasswordEncoder} implementation that uses PBKDF2 with :
+ * <ul>
+ * <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>
  * The algorithm is invoked on the concatenated bytes of the salt, secret and password.
  *
  * @author Rob Worsnop
@@ -44,11 +49,13 @@ import org.springframework.security.crypto.util.EncodingUtils;
  */
 public class Pbkdf2PasswordEncoder implements PasswordEncoder {
 
+	private static final int DEFAULT_SALT_LENGTH = 8;
+
 	private static final int DEFAULT_HASH_WIDTH = 256;
 
 	private static final int DEFAULT_ITERATIONS = 185000;
 
-	private final BytesKeyGenerator saltGenerator = KeyGenerators.secureRandom();
+	private final BytesKeyGenerator saltGenerator;
 
 	private final byte[] secret;
 
@@ -62,10 +69,10 @@ public class Pbkdf2PasswordEncoder implements PasswordEncoder {
 
 	/**
 	 * Constructs a PBKDF2 password encoder with no additional secret value. There will be
-	 * {@value DEFAULT_ITERATIONS} iterations and a hash width of
-	 * {@value DEFAULT_HASH_WIDTH}. 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.
+	 * 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.
 	 */
 	public Pbkdf2PasswordEncoder() {
 		this("");
@@ -73,24 +80,50 @@ public class Pbkdf2PasswordEncoder implements PasswordEncoder {
 
 	/**
 	 * Constructs a standard password encoder with a secret value which is also included
-	 * in the password hash. There will be {@value DEFAULT_ITERATIONS} iterations and a
-	 * hash width of {@value DEFAULT_HASH_WIDTH}.
+	 * 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.
 	 * @param secret the secret key used in the encoding process (should not be shared)
 	 */
 	public Pbkdf2PasswordEncoder(CharSequence secret) {
-		this(secret, DEFAULT_ITERATIONS, DEFAULT_HASH_WIDTH);
+		this(secret, DEFAULT_SALT_LENGTH, DEFAULT_ITERATIONS, DEFAULT_HASH_WIDTH);
+	}
+
+	/**
+	 * 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.
+	 * @param secret the secret
+	 * @param saltLength the salt length (in bytes)
+	 */
+	public Pbkdf2PasswordEncoder(CharSequence secret, int saltLength) {
+		this(secret, saltLength, DEFAULT_ITERATIONS, DEFAULT_HASH_WIDTH);
 	}
 
 	/**
 	 * Constructs a standard password encoder with a secret value as well as iterations
-	 * and hash.
+	 * and hash width. The salt length will be of {@value #DEFAULT_SALT_LENGTH} bytes.
 	 * @param secret the secret
 	 * @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
+	 * @param hashWidth the size of the hash (in bits)
 	 */
 	public Pbkdf2PasswordEncoder(CharSequence secret, int iterations, int hashWidth) {
+		this(secret, DEFAULT_SALT_LENGTH, iterations, hashWidth);
+	}
+
+	/**
+	 * Constructs a standard password encoder with a secret value as well as salt length,
+	 * iterations and hash width.
+	 * @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 hashWidth the size of the hash (in bits)
+	 */
+	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;
 	}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -24,11 +24,25 @@ import org.springframework.security.crypto.codec.Hex;
 import org.springframework.security.crypto.keygen.KeyGenerators;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
 
 public class Pbkdf2PasswordEncoderTests {
 
 	private Pbkdf2PasswordEncoder encoder = new Pbkdf2PasswordEncoder("secret");
 
+	private Pbkdf2PasswordEncoder encoderSalt16 = new Pbkdf2PasswordEncoder("", 16);
+
+	private Pbkdf2PasswordEncoder[] encoders = new Pbkdf2PasswordEncoder[] { this.encoder, this.encoderSalt16 };
+
+	@Test
+	public void encodedLengthSuccess() {
+		// encode output is an hex coded String so with 2 chars per encoding result byte
+		// (ie. 1 char for 4 bits).
+		// The encoding result size is : (saltLength * 8) bits + hashWith bits.
+		assertThat(this.encoder.encode("password").length()).isEqualTo((8 * 8 + 256) / 4);
+		assertThat(this.encoderSalt16.encode("password").length()).isEqualTo((16 * 8 + 256) / 4);
+	}
+
 	@Test
 	public void matches() {
 		String result = this.encoder.encode("password");
@@ -36,18 +50,37 @@ public class Pbkdf2PasswordEncoderTests {
 		assertThat(this.encoder.matches("password", result)).isTrue();
 	}
 
+	@Test
+	public void matchesWhenCustomSaltLengthThenSuccess() {
+		String result = this.encoderSalt16.encode("password");
+		assertThat(result.equals("password")).isFalse();
+		assertThat(this.encoderSalt16.matches("password", result)).isTrue();
+	}
+
 	@Test
 	public void matchesLengthChecked() {
 		String result = this.encoder.encode("password");
 		assertThat(this.encoder.matches("password", result.substring(0, result.length() - 2))).isFalse();
 	}
 
+	@Test
+	public void matchesLengthCheckedWhenCustomSaltLengthThenSuccess() {
+		String result = this.encoderSalt16.encode("password");
+		assertThat(this.encoderSalt16.matches("password", result.substring(0, result.length() - 2))).isFalse();
+	}
+
 	@Test
 	public void notMatches() {
 		String result = this.encoder.encode("password");
 		assertThat(this.encoder.matches("bogus", result)).isFalse();
 	}
 
+	@Test
+	public void notMatchesWhenCustomSaltLengthThenSuccess() {
+		String result = this.encoderSalt16.encode("password");
+		assertThat(this.encoderSalt16.matches("bogus", result)).isFalse();
+	}
+
 	@Test
 	public void encodeSamePasswordMultipleTimesDiffers() {
 		String password = "password";
@@ -56,6 +89,14 @@ public class Pbkdf2PasswordEncoderTests {
 		assertThat(encodeFirst).isNotEqualTo(encodeSecond);
 	}
 
+	@Test
+	public void encodeSamePasswordMultipleTimesWhenCustomSaltLengthThenDiffers() {
+		String password = "password";
+		String encodeFirst = this.encoderSalt16.encode(password);
+		String encodeSecond = this.encoderSalt16.encode(password);
+		assertThat(encodeFirst).isNotEqualTo(encodeSecond);
+	}
+
 	@Test
 	public void passivity() {
 		String encodedPassword = "ab1146a8458d4ce4e65789e5a3f60e423373cfa10b01abd23739e5ae2fdc37f8e9ede4ae6da65264";
@@ -63,6 +104,13 @@ public class Pbkdf2PasswordEncoderTests {
 		assertThat(this.encoder.matches(rawPassword, encodedPassword)).isTrue();
 	}
 
+	@Test
+	public void passivityWhenCustomSaltLengthThenSuccess() {
+		String encodedPassword = "0123456789abcdef0123456789abcdef10d883c2a0e653c97175c4a2583a7f1fd3301b319a7657d95f75365ea7c04ce1";
+		String rawPassword = "password";
+		assertThat(this.encoderSalt16.matches(rawPassword, encodedPassword)).isTrue();
+	}
+
 	@Test
 	public void migrate() {
 		final int saltLength = KeyGenerators.secureRandom().getKeyLength();
@@ -82,6 +130,24 @@ public class Pbkdf2PasswordEncoderTests {
 		assertThat(this.encoder.matches(rawPassword, encodedPassword)).isTrue();
 	}
 
+	@Test
+	public void encodeAndMatchWhenBase64AndCustomSaltLengthThenSuccess() {
+		this.encoderSalt16.setEncodeHashAsBase64(true);
+		String rawPassword = "password";
+		String encodedPassword = this.encoderSalt16.encode(rawPassword);
+		assertThat(this.encoderSalt16.matches(rawPassword, encodedPassword)).isTrue();
+	}
+
+	@Test
+	public void encodeWhenBase64ThenBase64DecodeSuccess() {
+		assertThat(this.encoders).allSatisfy((pe) -> {
+			pe.setEncodeHashAsBase64(true);
+			String encodedPassword = pe.encode("password");
+			// validate can decode as Base64
+			assertThatNoException().isThrownBy(() -> java.util.Base64.getDecoder().decode(encodedPassword));
+		});
+	}
+
 	@Test
 	public void matchWhenBase64ThenSuccess() {
 		this.encoder.setEncodeHashAsBase64(true);
@@ -92,6 +158,14 @@ public class Pbkdf2PasswordEncoderTests {
 																// Base64
 	}
 
+	@Test
+	public void matchWhenBase64AndCustomSaltLengthThenSuccess() {
+		this.encoderSalt16.setEncodeHashAsBase64(true);
+		String rawPassword = "password";
+		String encodedPassword = "ASNFZ4mrze8BI0VniavN7xDYg8Kg5lPJcXXEolg6fx/TMBsxmnZX2V91Nl6nwEzh";
+		assertThat(this.encoderSalt16.matches(rawPassword, encodedPassword)).isTrue();
+	}
+
 	@Test
 	public void encodeAndMatchWhenSha256ThenSuccess() {
 		this.encoder.setAlgorithm(Pbkdf2PasswordEncoder.SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA256);
@@ -100,6 +174,14 @@ public class Pbkdf2PasswordEncoderTests {
 		assertThat(this.encoder.matches(rawPassword, encodedPassword)).isTrue();
 	}
 
+	@Test
+	public void encodeAndMatchWhenSha256AndCustomSaltLengthThenSuccess() {
+		this.encoderSalt16.setAlgorithm(Pbkdf2PasswordEncoder.SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA256);
+		String rawPassword = "password";
+		String encodedPassword = this.encoderSalt16.encode(rawPassword);
+		assertThat(this.encoderSalt16.matches(rawPassword, encodedPassword)).isTrue();
+	}
+
 	@Test
 	public void matchWhenSha256ThenSuccess() {
 		this.encoder.setAlgorithm(Pbkdf2PasswordEncoder.SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA256);
@@ -108,6 +190,14 @@ public class Pbkdf2PasswordEncoderTests {
 		assertThat(this.encoder.matches(rawPassword, encodedPassword)).isTrue();
 	}
 
+	@Test
+	public void matchWhenSha256AndCustomSaltLengthThenSuccess() {
+		this.encoderSalt16.setAlgorithm(Pbkdf2PasswordEncoder.SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA256);
+		String rawPassword = "password";
+		String encodedPassword = "0123456789abcdef0123456789abcdefc7cfc96cd26b854d096ccbb3308fad860d719eb552ed52ef8352935539158287";
+		assertThat(this.encoderSalt16.matches(rawPassword, encodedPassword)).isTrue();
+	}
+
 	/**
 	 * Used to find the iteration count that takes .5 seconds.
 	 */