SEC-1666: Use constant time comparison for sensitive data.
Constant time comparison helps to mitigate timing attacks. See the following link for more information * http://rdist.root.org/2010/07/19/exploiting-remote-timing-attacks/ * http://en.wikipedia.org/wiki/Timing_attack for more information.
This commit is contained in:
Rob Winch
parent
6a62b51870
commit
8c08eeb57b
|
|
@ -145,7 +145,7 @@ public class LdapShaPasswordEncoder implements PasswordEncoder {
|
|||
|
||||
String encodedRawPass = encodePassword(rawPass, salt).substring(startOfHash);
|
||||
|
||||
return encodedRawPass.equals(encPass.substring(startOfHash));
|
||||
return PasswordEncoderUtils.equals(encodedRawPass,encPass.substring(startOfHash));
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -78,7 +78,7 @@ public class Md4PasswordEncoder extends BaseDigestPasswordEncoder {
|
|||
public boolean isPasswordValid(String encPass, String rawPass, Object salt) {
|
||||
String pass1 = "" + encPass;
|
||||
String pass2 = encodePassword(rawPass, salt);
|
||||
return pass1.equals(pass2);
|
||||
return PasswordEncoderUtils.equals(pass1,pass2);
|
||||
}
|
||||
|
||||
public String getAlgorithm() {
|
||||
|
|
|
|||
|
|
@ -126,7 +126,7 @@ public class MessageDigestPasswordEncoder extends BaseDigestPasswordEncoder {
|
|||
String pass1 = "" + encPass;
|
||||
String pass2 = encodePassword(rawPass, salt);
|
||||
|
||||
return pass1.equals(pass2);
|
||||
return PasswordEncoderUtils.equals(pass1,pass2);
|
||||
}
|
||||
|
||||
public String getAlgorithm() {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,45 @@
|
|||
package org.springframework.security.authentication.encoding;
|
||||
|
||||
import java.io.UnsupportedEncodingException;
|
||||
|
||||
/**
|
||||
* Utility for constant time comparison to prevent against timing attacks.
|
||||
*
|
||||
* @author Rob Winch
|
||||
*/
|
||||
class PasswordEncoderUtils {
|
||||
|
||||
/**
|
||||
* Constant time comparison to prevent against timing attacks.
|
||||
* @param expected
|
||||
* @param actual
|
||||
* @return
|
||||
*/
|
||||
static boolean equals(String expected, String actual) {
|
||||
byte[] expectedBytes = bytesUtf8(expected);
|
||||
byte[] actualBytes = bytesUtf8(actual);
|
||||
int expectedLength = expectedBytes == null ? -1 : expectedBytes.length;
|
||||
int actualLength = actualBytes == null ? -1 : actualBytes.length;
|
||||
if (expectedLength != actualLength) {
|
||||
return false;
|
||||
}
|
||||
|
||||
int result = 0;
|
||||
for (int i = 0; i < expectedLength; i++) {
|
||||
result |= expectedBytes[i] ^ actualBytes[i];
|
||||
}
|
||||
return result == 0;
|
||||
}
|
||||
|
||||
private static byte[] bytesUtf8(String s) {
|
||||
if(s == null) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
return s.getBytes("UTF-8");
|
||||
} catch (UnsupportedEncodingException e) {
|
||||
throw new IllegalStateException("Could not get bytes in UTF-8 format",e);
|
||||
}
|
||||
}
|
||||
private PasswordEncoderUtils() {}
|
||||
}
|
||||
|
|
@ -15,6 +15,8 @@
|
|||
|
||||
package org.springframework.security.authentication.encoding;
|
||||
|
||||
import java.util.Locale;
|
||||
|
||||
/**
|
||||
* <p>Plaintext implementation of PasswordEncoder.</p>
|
||||
* <P>As callers may wish to extract the password and salts separately from the encoded password, the salt must
|
||||
|
|
@ -46,11 +48,12 @@ public class PlaintextPasswordEncoder extends BasePasswordEncoder {
|
|||
// authentication will fail as the encodePassword never allows them)
|
||||
String pass2 = mergePasswordAndSalt(rawPass, salt, false);
|
||||
|
||||
if (!ignorePasswordCase) {
|
||||
return pass1.equals(pass2);
|
||||
} else {
|
||||
return pass1.equalsIgnoreCase(pass2);
|
||||
if (ignorePasswordCase) {
|
||||
// Note: per String javadoc to get correct results for Locale insensitive, use English
|
||||
pass1 = pass1.toLowerCase(Locale.ENGLISH);
|
||||
pass2 = pass2.toLowerCase(Locale.ENGLISH);
|
||||
}
|
||||
return PasswordEncoderUtils.equals(pass1,pass2);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -0,0 +1,33 @@
|
|||
package org.springframework.security.authentication.encoding;
|
||||
|
||||
import static org.junit.Assert.*;
|
||||
|
||||
import org.junit.Test;
|
||||
/**
|
||||
* @author Rob Winch
|
||||
*/
|
||||
public class PasswordEncoderUtilsTests {
|
||||
|
||||
@Test
|
||||
public void differentLength() {
|
||||
assertFalse(PasswordEncoderUtils.equals("abc", "a"));
|
||||
assertFalse(PasswordEncoderUtils.equals("a", "abc"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void equalsNull() {
|
||||
assertFalse(PasswordEncoderUtils.equals(null, "a"));
|
||||
assertFalse(PasswordEncoderUtils.equals("a", null));
|
||||
assertTrue(PasswordEncoderUtils.equals(null, null));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void equalsCaseSensitive() {
|
||||
assertFalse(PasswordEncoderUtils.equals("aBc", "abc"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void equalsSuccess() {
|
||||
assertTrue(PasswordEncoderUtils.equals("abcdef", "abcdef"));
|
||||
}
|
||||
}
|
||||
|
|
@ -21,8 +21,6 @@ import static org.springframework.security.crypto.util.EncodingUtils.hexEncode;
|
|||
import static org.springframework.security.crypto.util.EncodingUtils.subArray;
|
||||
import static org.springframework.security.crypto.util.EncodingUtils.utf8Encode;
|
||||
|
||||
import java.util.Arrays;
|
||||
|
||||
import org.springframework.security.crypto.keygen.BytesKeyGenerator;
|
||||
import org.springframework.security.crypto.keygen.KeyGenerators;
|
||||
import org.springframework.security.crypto.util.Digester;
|
||||
|
|
@ -79,8 +77,21 @@ public final class StandardPasswordEncoder implements PasswordEncoder {
|
|||
return hexDecode(encodedPassword);
|
||||
}
|
||||
|
||||
/**
|
||||
* Constant time comparison to prevent against timing attacks.
|
||||
* @param expected
|
||||
* @param actual
|
||||
* @return
|
||||
*/
|
||||
private boolean matches(byte[] expected, byte[] actual) {
|
||||
return Arrays.equals(expected, 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;
|
||||
}
|
||||
}
|
||||
|
|
@ -16,6 +16,12 @@ public class StandardPasswordEncoderTests {
|
|||
assertTrue(encoder.matches("password", result));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void matchesLengthChecked() {
|
||||
String result = encoder.encode("password");
|
||||
assertFalse(encoder.matches("password", result.substring(0,result.length()-1)));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void notMatches() {
|
||||
String result = encoder.encode("password");
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ import org.springframework.util.StringUtils;
|
|||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
|
||||
import java.io.UnsupportedEncodingException;
|
||||
import java.security.MessageDigest;
|
||||
import java.security.NoSuchAlgorithmException;
|
||||
import java.util.Arrays;
|
||||
|
|
@ -81,6 +82,7 @@ public class TokenBasedRememberMeServices extends AbstractRememberMeServices {
|
|||
|
||||
//~ Methods ========================================================================================================
|
||||
|
||||
@Override
|
||||
protected UserDetails processAutoLoginCookie(String[] cookieTokens, HttpServletRequest request,
|
||||
HttpServletResponse response) {
|
||||
|
||||
|
|
@ -117,9 +119,9 @@ public class TokenBasedRememberMeServices extends AbstractRememberMeServices {
|
|||
String expectedTokenSignature = makeTokenSignature(tokenExpiryTime, userDetails.getUsername(),
|
||||
userDetails.getPassword());
|
||||
|
||||
if (!expectedTokenSignature.equals(cookieTokens[2])) {
|
||||
if (!equals(expectedTokenSignature,cookieTokens[2])) {
|
||||
throw new InvalidCookieException("Cookie token[2] contained signature '" + cookieTokens[2]
|
||||
+ "' but expected '" + expectedTokenSignature + "'");
|
||||
+ "' but expected '" + expectedTokenSignature + "'");
|
||||
}
|
||||
|
||||
return userDetails;
|
||||
|
|
@ -145,6 +147,7 @@ public class TokenBasedRememberMeServices extends AbstractRememberMeServices {
|
|||
return tokenExpiryTime < System.currentTimeMillis();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onLoginSuccess(HttpServletRequest request, HttpServletResponse response,
|
||||
Authentication successfulAuthentication) {
|
||||
|
||||
|
|
@ -216,4 +219,35 @@ public class TokenBasedRememberMeServices extends AbstractRememberMeServices {
|
|||
private boolean isInstanceOfUserDetails(Authentication authentication) {
|
||||
return authentication.getPrincipal() instanceof UserDetails;
|
||||
}
|
||||
|
||||
/**
|
||||
* Constant time comparison to prevent against timing attacks.
|
||||
* @param expected
|
||||
* @param actual
|
||||
* @return
|
||||
*/
|
||||
private static boolean equals(String expected, String actual) {
|
||||
byte[] expectedBytes = bytesUtf8(expected);
|
||||
byte[] actualBytes = bytesUtf8(actual);
|
||||
if (expectedBytes.length != actualBytes.length) {
|
||||
return false;
|
||||
}
|
||||
|
||||
int result = 0;
|
||||
for (int i = 0; i < expectedBytes.length; i++) {
|
||||
result |= expectedBytes[i] ^ actualBytes[i];
|
||||
}
|
||||
return result == 0;
|
||||
}
|
||||
|
||||
private static byte[] bytesUtf8(String s) {
|
||||
if(s == null) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
return s.getBytes("UTF-8");
|
||||
} catch (UnsupportedEncodingException e) {
|
||||
throw new IllegalStateException("Could not get bytes in UTF-8 format",e);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue