From a2858240f10d0a91ac351a2764330f9682e27b61 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Wed, 27 Apr 2011 17:34:54 +0100 Subject: [PATCH] SEC-1728: Remove references to SUN provider and incorrect seeding of SecureRandom in SecureRandomBytesKeyGenerator. --- .../keygen/SecureRandomBytesKeyGenerator.java | 40 +++++-------------- .../security/crypto/password/Digester.java | 14 ++++--- .../password/StandardPasswordEncoder.java | 21 ++++++++-- .../crypto/password/DigesterTests.java | 24 ++++------- 4 files changed, 41 insertions(+), 58 deletions(-) diff --git a/core/src/main/java/org/springframework/security/crypto/keygen/SecureRandomBytesKeyGenerator.java b/core/src/main/java/org/springframework/security/crypto/keygen/SecureRandomBytesKeyGenerator.java index eebb73733e..196df3de61 100644 --- a/core/src/main/java/org/springframework/security/crypto/keygen/SecureRandomBytesKeyGenerator.java +++ b/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. + *

+ * 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; -} \ No newline at end of file +} diff --git a/core/src/main/java/org/springframework/security/crypto/password/Digester.java b/core/src/main/java/org/springframework/security/crypto/password/Digester.java index 01935f8a4b..a74d58cd66 100644 --- a/core/src/main/java/org/springframework/security/crypto/password/Digester.java +++ b/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) { diff --git a/core/src/main/java/org/springframework/security/crypto/password/StandardPasswordEncoder.java b/core/src/main/java/org/springframework/security/crypto/password/StandardPasswordEncoder.java index 72e6b7cad7..96651d3749 100644 --- a/core/src/main/java/org/springframework/security/crypto/password/StandardPasswordEncoder.java +++ b/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; + } diff --git a/core/src/test/java/org/springframework/security/crypto/password/DigesterTests.java b/core/src/test/java/org/springframework/security/crypto/password/DigesterTests.java index 97f14f06bb..56c7e4baa7 100644 --- a/core/src/test/java/org/springframework/security/crypto/password/DigesterTests.java +++ b/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)); - } }