diff --git a/core/src/main/java/org/springframework/security/providers/ldap/authenticator/LdapShaPasswordEncoder.java b/core/src/main/java/org/springframework/security/providers/ldap/authenticator/LdapShaPasswordEncoder.java index bf4bcd5a5f..6556c58385 100644 --- a/core/src/main/java/org/springframework/security/providers/ldap/authenticator/LdapShaPasswordEncoder.java +++ b/core/src/main/java/org/springframework/security/providers/ldap/authenticator/LdapShaPasswordEncoder.java @@ -15,8 +15,6 @@ package org.springframework.security.providers.ldap.authenticator; -import org.springframework.security.ldap.LdapDataAccessException; - import org.springframework.security.providers.encoding.PasswordEncoder; import org.springframework.security.providers.encoding.ShaPasswordEncoder; @@ -85,7 +83,7 @@ public class LdapShaPasswordEncoder implements PasswordEncoder { try { sha = MessageDigest.getInstance("SHA"); } catch (java.security.NoSuchAlgorithmException e) { - throw new LdapDataAccessException("No SHA implementation available!", e); + throw new IllegalStateException("No SHA implementation available!", e); } sha.update(rawPass.getBytes()); @@ -129,23 +127,44 @@ public class LdapShaPasswordEncoder implements PasswordEncoder { * * @return true if they match (independent of the case of the prefix). */ - public boolean isPasswordValid(String encPass, String rawPass, Object salt) { - String encPassWithoutPrefix; - - if (!encPass.startsWith("{")) { + public boolean isPasswordValid(final String encPass, final String rawPass, Object salt) { + String prefix = extractPrefix(encPass); + + if (prefix == null) { return encPass.equals(rawPass); } - if (encPass.startsWith(SSHA_PREFIX) || encPass.startsWith(SSHA_PREFIX_LC)) { - encPassWithoutPrefix = encPass.substring(6); + if (prefix.equals(SSHA_PREFIX) || prefix.equals(SSHA_PREFIX_LC)) { salt = extractSalt(encPass); + } else if (!prefix.equals(SHA_PREFIX) && !prefix.equals(SHA_PREFIX_LC)) { + throw new IllegalArgumentException("Unsupported password prefix '" + prefix + "'"); } else { - encPassWithoutPrefix = encPass.substring(5); - salt = null; + // Standard SHA + salt = null; } - // Compare the encoded passwords without the prefix - return encodePassword(rawPass, salt).endsWith(encPassWithoutPrefix); + int startOfHash = prefix.length() + 1; + + String encodedRawPass = encodePassword(rawPass, salt).substring(startOfHash); + + return encodedRawPass.equals(encPass.substring(startOfHash)); + } + + /** + * Returns the hash prefix or null if there isn't one. + */ + private String extractPrefix(String encPass) { + if (!encPass.startsWith("{")) { + return null; + } + + int secondBrace = encPass.lastIndexOf('}'); + + if (secondBrace < 0) { + throw new IllegalArgumentException("Couldn't find closing brace for SHA prefix"); + } + + return encPass.substring(0, secondBrace + 1); } public void setForceLowerCasePrefix(boolean forceLowerCasePrefix) { diff --git a/core/src/main/java/org/springframework/security/providers/ldap/authenticator/PasswordComparisonAuthenticator.java b/core/src/main/java/org/springframework/security/providers/ldap/authenticator/PasswordComparisonAuthenticator.java index cf9d17ad70..1fb039b09d 100644 --- a/core/src/main/java/org/springframework/security/providers/ldap/authenticator/PasswordComparisonAuthenticator.java +++ b/core/src/main/java/org/springframework/security/providers/ldap/authenticator/PasswordComparisonAuthenticator.java @@ -35,17 +35,11 @@ import java.util.Iterator; /** * An {@link org.springframework.security.providers.ldap.LdapAuthenticator LdapAuthenticator} which compares the login - * password with the value stored in the directory using an LDAP "compare" operation. + * password with the value stored in the directory using a remote LDAP "compare" operation. * *

- * This can be achieved either by retrieving the password attribute for the user and comparing it locally, - * or by peforming an LDAP "compare" operation. If the password attribute (default "userPassword") is found in the - * retrieved attributes it will be compared locally. If not, the remote comparison will be attempted. - *

- *

* If passwords are stored in digest form in the repository, then a suitable {@link PasswordEncoder} * implementation must be supplied. By default, passwords are encoded using the {@link LdapShaPasswordEncoder}. - *

* * @author Luke Taylor * @version $Id$ diff --git a/core/src/test/java/org/springframework/security/providers/ldap/authenticator/LdapShaPasswordEncoderTests.java b/core/src/test/java/org/springframework/security/providers/ldap/authenticator/LdapShaPasswordEncoderTests.java index 9ea43f0603..82e1bafd26 100644 --- a/core/src/test/java/org/springframework/security/providers/ldap/authenticator/LdapShaPasswordEncoderTests.java +++ b/core/src/test/java/org/springframework/security/providers/ldap/authenticator/LdapShaPasswordEncoderTests.java @@ -15,7 +15,10 @@ package org.springframework.security.providers.ldap.authenticator; -import junit.framework.TestCase; +import static org.junit.Assert.*; + +import org.junit.Before; +import org.junit.Test; /** @@ -24,37 +27,39 @@ import junit.framework.TestCase; * @author Luke Taylor * @version $Id$ */ -public class LdapShaPasswordEncoderTests extends TestCase { +public class LdapShaPasswordEncoderTests { //~ Instance fields ================================================================================================ LdapShaPasswordEncoder sha; //~ Methods ======================================================================================================== - protected void setUp() throws Exception { - super.setUp(); + @Before + public void setUp() throws Exception { sha = new LdapShaPasswordEncoder(); } - public void testInvalidPasswordFails() { + @Test + public void invalidPasswordFails() { assertFalse(sha.isPasswordValid("{SHA}ddSFGmjXYPbZC+NXR2kCzBRjqiE=", "wrongpassword", null)); } - public void testInvalidSaltedPasswordFails() { + @Test + public void invalidSaltedPasswordFails() { assertFalse(sha.isPasswordValid("{SSHA}25ro4PKC8jhQZ26jVsozhX/xaP0suHgX", "wrongpassword", null)); assertFalse(sha.isPasswordValid("{SSHA}PQy2j+6n5ytA+YlAKkM8Fh4p6u2JxfVd", "wrongpassword", null)); } - public void testNonByteArraySaltThrowsException() { - try { - sha.encodePassword("password", "AStringNotAByteArray"); - } catch (IllegalArgumentException expected) {} + @Test(expected=IllegalArgumentException.class) + public void nonByteArraySaltThrowsException() { + sha.encodePassword("password", "AStringNotAByteArray"); } /** * Test values generated by 'slappasswd -h {SHA} -s boabspasswurd' */ - public void testValidPasswordSucceeds() { + @Test + public void validPasswordSucceeds() { sha.setForceLowerCasePrefix(false); assertTrue(sha.isPasswordValid("{SHA}ddSFGmjXYPbZC+NXR2kCzBRjqiE=", "boabspasswurd", null)); assertTrue(sha.isPasswordValid("{sha}ddSFGmjXYPbZC+NXR2kCzBRjqiE=", "boabspasswurd", null)); @@ -66,7 +71,8 @@ public class LdapShaPasswordEncoderTests extends TestCase { /** * Test values generated by 'slappasswd -s boabspasswurd' */ - public void testValidSaltedPasswordSucceeds() { + @Test + public void validSaltedPasswordSucceeds() { sha.setForceLowerCasePrefix(false); assertTrue(sha.isPasswordValid("{SSHA}25ro4PKC8jhQZ26jVsozhX/xaP0suHgX", "boabspasswurd", null)); assertTrue(sha.isPasswordValid("{ssha}PQy2j+6n5ytA+YlAKkM8Fh4p6u2JxfVd", "boabspasswurd", null)); @@ -75,7 +81,8 @@ public class LdapShaPasswordEncoderTests extends TestCase { assertTrue(sha.isPasswordValid("{ssha}PQy2j+6n5ytA+YlAKkM8Fh4p6u2JxfVd", "boabspasswurd", null)); } - public void testCorrectPrefixCaseIsUsed() { + @Test + public void correctPrefixCaseIsUsed() { sha.setForceLowerCasePrefix(false); assertEquals("{SHA}ddSFGmjXYPbZC+NXR2kCzBRjqiE=", sha.encodePassword("boabspasswurd", null)); assertTrue(sha.encodePassword("somepassword", "salt".getBytes()).startsWith("{SSHA}")); @@ -85,4 +92,15 @@ public class LdapShaPasswordEncoderTests extends TestCase { assertTrue(sha.encodePassword("somepassword", "salt".getBytes()).startsWith("{ssha}")); } + + @Test(expected=IllegalArgumentException.class) + public void invalidPrefixIsRejected() { + sha.isPasswordValid("{MD9}xxxxxxxxxx" , "somepassword", null); + } + + @Test(expected=IllegalArgumentException.class) + public void malformedPrefixIsRejected() { + // No right brace + sha.isPasswordValid("{SSHA25ro4PKC8jhQZ26jVsozhX/xaP0suHgX" , "somepassword", null); + } }