SEC-1728: Remove references to SUN provider and incorrect seeding of SecureRandom in SecureRandomBytesKeyGenerator.

core/src/main/java/org/springframework/security/crypto/keygen/SecureRandomBytesKeyGenerator.java

@@ -20,8 +20,11 @@ import java.security.NoSuchProviderException;
 import java.security.SecureRandom;
 
 /**
- * A KeyGenerator that uses SecureRandom to generate byte array-based keys.
- * Defaults to 8 byte keys produced by the SHA1PRNG algorithm developed by the Sun Provider.
+ * A KeyGenerator that uses {@link SecureRandom} to generate byte array-based keys.
+ * <p>
+ * No specific provider is used for the {@code SecureRandom}, so the platform default
+ * will be used.
+ *
  * @author Keith Donald
  */
 final class SecureRandomBytesKeyGenerator implements BytesKeyGenerator {
@@ -34,14 +37,15 @@ final class SecureRandomBytesKeyGenerator implements BytesKeyGenerator {
      * Creates a secure random key generator using the defaults.
      */
     public SecureRandomBytesKeyGenerator() {
-        this(DEFAULT_ALGORITHM, DEFAULT_PROVIDER, DEFAULT_KEY_LENGTH);
+        this(DEFAULT_KEY_LENGTH);
     }
 
     /**
      * Creates a secure random key generator with a custom key length.
      */
     public SecureRandomBytesKeyGenerator(int keyLength) {
-        this(DEFAULT_ALGORITHM, DEFAULT_PROVIDER, keyLength);
+        this.random = new SecureRandom();
+        this.keyLength = keyLength;
     }
 
     public int getKeyLength() {
@@ -54,32 +58,6 @@ final class SecureRandomBytesKeyGenerator implements BytesKeyGenerator {
         return bytes;
     }
 
-    // internal helpers
-
-    /**
-     * Creates a secure random key generator that is fully customized.
-     */
-    private SecureRandomBytesKeyGenerator(String algorithm, String provider, int keyLength) {
-        this.random = createSecureRandom(algorithm, provider, keyLength);
-        this.keyLength = keyLength;
-    }
-
-    private SecureRandom createSecureRandom(String algorithm, String provider, int keyLength) {
-        try {
-            SecureRandom random = SecureRandom.getInstance(algorithm, provider);
-            random.setSeed(random.generateSeed(keyLength));
-            return random;
-        } catch (NoSuchAlgorithmException e) {
-            throw new IllegalArgumentException("Not a supported SecureRandom key generation algorithm", e);
-        } catch (NoSuchProviderException e) {
-            throw new IllegalArgumentException("Not a supported SecureRandom key provider", e);
-        }
-    }
-
-    private static final String DEFAULT_ALGORITHM = "SHA1PRNG";
-
-    private static final String DEFAULT_PROVIDER = "SUN";
-
     private static final int DEFAULT_KEY_LENGTH = 8;
 
-}
+}

core/src/main/java/org/springframework/security/crypto/password/Digester.java

@@ -21,28 +21,30 @@ import java.security.NoSuchProviderException;
 
 /**
  * Helper for working with the MessageDigest API.
+ *
  * Performs 1024 iterations of the hashing algorithm per digest to aid in protecting against brute force attacks.
+ *
  * @author Keith Donald
+ * @author Luke Taylor
  */
 class Digester {
 
     private final MessageDigest messageDigest;
 
-    private final int iterations = 1024;
+    private final int iterations;
 
     /**
      * Create a new Digester.
      * @param algorithm the digest algorithm; for example, "SHA-1" or "SHA-256".
-     * @param provider the provider of the digest algorithm, for example "SUN".
      */
-    public Digester(String algorithm, String provider) {
+    public Digester(String algorithm, int iterations) {
         try {
-            messageDigest = MessageDigest.getInstance(algorithm, provider);
+            messageDigest = MessageDigest.getInstance(algorithm);
         } catch (NoSuchAlgorithmException e) {
             throw new IllegalStateException("No such hashing algorithm", e);
-        } catch (NoSuchProviderException e) {
-            throw new IllegalStateException("No such provider for hashing algorithm", e);
         }
+
+        this.iterations = iterations;
     }
 
     public byte[] digest(byte[] value) {

core/src/main/java/org/springframework/security/crypto/password/StandardPasswordEncoder.java

@@ -30,6 +30,7 @@ import org.springframework.security.crypto.keygen.KeyGenerators;
  * The digest algorithm is invoked on the concatenated bytes of the salt, secret and password.
  *
  * @author Keith Donald
+ * @author Luke Taylor
  */
 public final class StandardPasswordEncoder implements PasswordEncoder {
 
@@ -40,11 +41,20 @@ public final class StandardPasswordEncoder implements PasswordEncoder {
     private final BytesKeyGenerator saltGenerator;
 
     /**
-     * Constructs a standard password encoder.
+     * Constructs a standard password encoder with no additional secret value.
+     */
+    public StandardPasswordEncoder() {
+        this("");
+    }
+
+    /**
+     * Constructs a standard password encoder with a secret value which is also included in the
+     * password hash.
+     *
      * @param secret the secret key used in the encoding process (should not be shared)
      */
     public StandardPasswordEncoder(CharSequence secret) {
-        this("SHA-256", "SUN", secret);
+        this("SHA-256", secret);
     }
 
     public String encode(CharSequence rawPassword) {
@@ -59,8 +69,8 @@ public final class StandardPasswordEncoder implements PasswordEncoder {
 
     // internal helpers
 
-    private StandardPasswordEncoder(String algorithm, String provider, CharSequence secret) {
-        this.digester = new Digester(algorithm, provider);
+    private StandardPasswordEncoder(String algorithm, CharSequence secret) {
+        this.digester = new Digester(algorithm, DEFAULT_ITERATIONS);
         this.secret = Utf8.encode(secret);
         this.saltGenerator = KeyGenerators.secureRandom();
     }
@@ -93,4 +103,7 @@ public final class StandardPasswordEncoder implements PasswordEncoder {
         }
         return result == 0;
     }
+
+    private static final int DEFAULT_ITERATIONS = 1024;
+
 }

core/src/test/java/org/springframework/security/crypto/password/DigesterTests.java

@@ -7,28 +7,18 @@ import java.security.MessageDigest;
 import java.util.Arrays;
 
 import org.junit.Test;
+import org.springframework.security.crypto.codec.Hex;
+import org.springframework.security.crypto.codec.Utf8;
 import org.springframework.security.crypto.password.Digester;
 
 public class DigesterTests {
 
-    private Digester digester = new Digester("SHA-1", "SUN");
-
     @Test
-    public void digest() {
-        byte[] result = digester.digest("text".getBytes());
-        assertEquals(20, result.length);
-        assertFalse(new String(result).equals("text"));
+    public void digestIsCorrectFor2Iterations() {
+        Digester digester = new Digester("SHA-1", 2);
+        byte[] result = digester.digest(Utf8.encode("text"));
+        // echo -n text | openssl sha1 -binary | openssl sha1
+        assertEquals("cdcefc6a573f294e60e1d633bca3aeba450954a3", new String(Hex.encode(result)));
     }
 
-    @Test
-    public void multiPassDigest() throws Exception {
-        MessageDigest d = MessageDigest.getInstance("SHA-1","SUN");
-        d.reset();
-        byte[] value = "text".getBytes("UTF-8");
-        byte[] singlePass = d.digest(value);
-        byte[] multiPass = digester.digest(value);
-        assertFalse(Arrays.toString(singlePass) + " should not be equal to "
-                + Arrays.toString(multiPass),
-                Arrays.equals(singlePass, multiPass));
-    }
 }