From ad489495dc87253fdd51995c7b5732683ddeec69 Mon Sep 17 00:00:00 2001 From: Loic Guibert Date: Sat, 24 Oct 2020 11:27:26 +0200 Subject: [PATCH] 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 --- .../password/Pbkdf2PasswordEncoder.java | 65 +++++++++---- .../password/Pbkdf2PasswordEncoderTests.java | 92 ++++++++++++++++++- 2 files changed, 140 insertions(+), 17 deletions(-) diff --git a/crypto/src/main/java/org/springframework/security/crypto/password/Pbkdf2PasswordEncoder.java b/crypto/src/main/java/org/springframework/security/crypto/password/Pbkdf2PasswordEncoder.java index 73e660417c..12722a206c 100644 --- a/crypto/src/main/java/org/springframework/security/crypto/password/Pbkdf2PasswordEncoder.java +++ b/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. - *

- * The width of the output hash can also be configured. - *

+ * A {@link PasswordEncoder} implementation that uses PBKDF2 with : + *

* 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; } diff --git a/crypto/src/test/java/org/springframework/security/crypto/password/Pbkdf2PasswordEncoderTests.java b/crypto/src/test/java/org/springframework/security/crypto/password/Pbkdf2PasswordEncoderTests.java index bd54171718..3ed919fa75 100644 --- a/crypto/src/test/java/org/springframework/security/crypto/password/Pbkdf2PasswordEncoderTests.java +++ b/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. */