Polish Pbkdf2PasswordEncoder

Fixes gh-2158
Fixes gh-51

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

@@ -20,9 +20,13 @@ import java.security.GeneralSecurityException;
 import javax.crypto.SecretKeyFactory;
 import javax.crypto.spec.PBEKeySpec;
 
+import org.springframework.security.crypto.codec.Hex;
 import org.springframework.security.crypto.codec.Utf8;
+import org.springframework.security.crypto.keygen.BytesKeyGenerator;
+import org.springframework.security.crypto.keygen.KeyGenerators;
 
 import static org.springframework.security.crypto.util.EncodingUtils.concatenate;
+import static org.springframework.security.crypto.util.EncodingUtils.subArray;
 
 /**
  * A {@code PasswordEncoder} implementation that uses PBKDF2 with a configurable number of
@@ -33,11 +37,15 @@ import static org.springframework.security.crypto.util.EncodingUtils.concatenate
  * The algorithm is invoked on the concatenated bytes of the salt, secret and password.
  *
  * @author Rob Worsnop
+ * @author Rob Winch
+ * @since 4.1
  */
-public class PBKDF2PasswordEncoder extends AbstractPasswordEncoder {
+public class Pbkdf2PasswordEncoder implements PasswordEncoder {
 	private static final String PBKDF2_ALGORITHM = "PBKDF2WithHmacSHA1";
-	private static final int DEFAULT_HASH_WIDTH = 160;
-	private static final int DEFAULT_ITERATIONS = 1024;
+	private static final int DEFAULT_HASH_WIDTH = 256;
+	private static final int DEFAULT_ITERATIONS = 185000;
+
+	private final BytesKeyGenerator saltGenerator = KeyGenerators.secureRandom();
 
 	private final byte[] secret;
 	private final int hashWidth;
@@ -45,9 +53,11 @@ public class PBKDF2PasswordEncoder extends AbstractPasswordEncoder {
 
 	/**
 	 * Constructs a PBKDF2 password encoder with no additional secret value. There will be
-	 * 1024 iterations and a hash width of 160.
+	 * 360000 iterations and a hash width of 160. 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() {
+	public Pbkdf2PasswordEncoder() {
 		this("");
 	}
 
@@ -57,7 +67,7 @@ public class PBKDF2PasswordEncoder extends AbstractPasswordEncoder {
 	 *
 	 * @param secret the secret key used in the encoding process (should not be shared)
 	 */
-	public PBKDF2PasswordEncoder(CharSequence secret) {
+	public Pbkdf2PasswordEncoder(CharSequence secret) {
 		this(secret, DEFAULT_ITERATIONS, DEFAULT_HASH_WIDTH);
 	}
 
@@ -65,18 +75,51 @@ public class PBKDF2PasswordEncoder extends AbstractPasswordEncoder {
 	 * Constructs a standard password encoder with a secret value as well as iterations
 	 * and hash.
 	 *
-	 * @param secret
-	 * @param iterations
-	 * @param hashWidth
+	 * @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
 	 */
-	public PBKDF2PasswordEncoder(CharSequence secret, int iterations, int hashWidth) {
+	public Pbkdf2PasswordEncoder(CharSequence secret, int iterations, int hashWidth) {
 		this.secret = Utf8.encode(secret);
 		this.iterations = iterations;
 		this.hashWidth = hashWidth;
 	}
 
 	@Override
-	protected byte[] encode(CharSequence rawPassword, byte[] salt) {
+	public String encode(CharSequence rawPassword) {
+		byte[] salt = this.saltGenerator.generateKey();
+		byte[] encoded = encodeAndConcatenate(rawPassword, salt);
+		return String.valueOf(Hex.encode(encoded));
+	}
+
+	@Override
+	public boolean matches(CharSequence rawPassword, String encodedPassword) {
+		byte[] digested = Hex.decode(encodedPassword);
+		byte[] salt = subArray(digested, 0, this.saltGenerator.getKeyLength());
+		return matches(digested, encodeAndConcatenate(rawPassword, salt));
+	}
+
+	private byte[] encodeAndConcatenate(CharSequence rawPassword, byte[] salt) {
+		return concatenate(salt, encode(rawPassword, salt));
+	}
+
+	/**
+	 * Constant time comparison to prevent against timing attacks.
+	 */
+	private static boolean matches(byte[] expected, byte[] actual) {
+		if (expected.length != actual.length) {
+			return false;
+		}
+
+		int result = 0;
+		for (int i = 0; i < expected.length; i++) {
+			result |= expected[i] ^ actual[i];
+		}
+		return result == 0;
+	}
+
+	private byte[] encode(CharSequence rawPassword, byte[] salt) {
 		try {
 			PBEKeySpec spec = new PBEKeySpec(rawPassword.toString().toCharArray(),
 					concatenate(salt, this.secret), this.iterations, this.hashWidth);
@@ -87,4 +130,4 @@ public class PBKDF2PasswordEncoder extends AbstractPasswordEncoder {
 			throw new IllegalStateException("Could not create hash", e);
 		}
 	}
-}
+}

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

@@ -1,31 +0,0 @@
-package org.springframework.security.crypto.password;
-
-import org.junit.Test;
-
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
-
-public class PBKDF2PasswordEncoderTests {
-    private PBKDF2PasswordEncoder encoder = new PBKDF2PasswordEncoder("secret");
-
-    @Test
-    public void matches() {
-        String result = encoder.encode("password");
-        assertFalse(result.equals("password"));
-        assertTrue(encoder.matches("password", result));
-    }
-
-    @Test
-    public void matchesLengthChecked() {
-        String result = encoder.encode("password");
-        assertFalse(encoder.matches("password", result.substring(0,result.length()-2)));
-    }
-
-    @Test
-    public void notMatches() {
-        String result = encoder.encode("password");
-        assertFalse(encoder.matches("bogus", result));
-    }
-
-}

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

@@ -0,0 +1,83 @@
+/*
+ * Copyright 2002-2015 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.security.crypto.password;
+
+import org.junit.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class Pbkdf2PasswordEncoderTests {
+	private Pbkdf2PasswordEncoder encoder = new Pbkdf2PasswordEncoder("secret");
+
+	@Test
+	public void matches() {
+		String result = this.encoder.encode("password");
+		assertThat(result.equals("password")).isFalse();
+		assertThat(this.encoder.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 notMatches() {
+		String result = this.encoder.encode("password");
+		assertThat(this.encoder.matches("bogus", result)).isFalse();
+	}
+
+	@Test
+	public void encodeSamePasswordMultipleTimesDiffers() {
+		String password = "password";
+		String encodeFirst = this.encoder.encode(password);
+		String encodeSecond = this.encoder.encode(password);
+		assertThat(encodeFirst).isNotEqualTo(encodeSecond);
+	}
+
+	/**
+	 * Used to find the iteration count that takes .5 seconds.
+	 */
+	public void findDefaultIterationCount() {
+		// warm up
+		run(180000, 10);
+		// find the default
+		run(165000, 10);
+	}
+
+	private void run(int iterations, int count) {
+		long HALF_SECOND = 500L;
+		long avg = 0;
+		while (avg < HALF_SECOND) {
+			iterations += 10000;
+			Pbkdf2PasswordEncoder encoder = new Pbkdf2PasswordEncoder("", iterations,
+					256);
+			String encoded = encoder.encode("password");
+			System.out.println("Trying " + iterations);
+			long start = System.currentTimeMillis();
+			for (int i = 0; i < count; i++) {
+				encoder.matches("password", encoded);
+			}
+			long end = System.currentTimeMillis();
+			long diff = end - start;
+			avg = diff / count;
+			System.out.println("Avgerage " + avg);
+		}
+		System.out.println("Iterations " + iterations);
+	}
+}

docs/manual/src/docs/asciidoc/index.adoc

@@ -381,6 +381,7 @@ You can find the highlights below:
 * <<csrf-cookie,CookieCsrfTokenRepository>> provides simple AngularJS & CSRF integration
 * Added `ForwardAuthenticationFailureHandler` & `ForwardAuthenticationSuccessHandler`
 * SCrypt support with `SCryptPasswordEncoder`
+* PBKDF2 support with <<spring-security-crypto-passwordencoders,Pbkdf2PasswordEncoder>>
 * Meta Annotation Support
 ** <<test-method-meta-annotations,Test Meta Annotations>>
 ** <<method-security-meta-annotations,Method Security Meta Annotations>>
@@ -6401,6 +6402,19 @@ String result = encoder.encode("myPassword");
 assertTrue(encoder.matches("myPassword", result));
 ----
 
+The `Pbkdf2PasswordEncoder` implementation uses PBKDF2 algorithm to hash the passwords.
+In order to defeat password cracking PBKDF2 is a deliberately slow algorithm and should be tuned to take about .5 seconds to verify a password on your system.
+
+
+[source,java]
+----
+
+// Create an encoder with all the defaults
+Pbkdf2PasswordEncoder encoder = new Pbkdf2PasswordEncoder();
+String result = encoder.encode("myPassword");
+assertTrue(encoder.matches("myPassword", result));
+----
+
 [[concurrency]]
 == Concurrency Support