Use MessageDigest.isEqual() where possible

fixes #7058
This commit is contained in:
Lars Grefer 2019-07-03 00:23:33 +02:00 committed by Josh Cummings
parent cd54808718
commit 4b0fb19fff
7 changed files with 17 additions and 79 deletions

View File

@ -14,6 +14,8 @@
package org.springframework.security.crypto.bcrypt;
import java.io.UnsupportedEncodingException;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.util.Arrays;
import java.security.SecureRandom;
@ -912,17 +914,6 @@ public class BCrypt {
}
static boolean equalsNoEarlyReturn(String a, String b) {
char[] caa = a.toCharArray();
char[] cab = b.toCharArray();
if (caa.length != cab.length) {
return false;
}
byte ret = 0;
for (int i = 0; i < caa.length; i++) {
ret |= caa[i] ^ cab[i];
}
return ret == 0;
return MessageDigest.isEqual(a.getBytes(StandardCharsets.UTF_8), b.getBytes(StandardCharsets.UTF_8));
}
}

View File

@ -19,6 +19,8 @@ import org.springframework.security.crypto.codec.Hex;
import org.springframework.security.crypto.keygen.BytesKeyGenerator;
import org.springframework.security.crypto.keygen.KeyGenerators;
import java.security.MessageDigest;
import static org.springframework.security.crypto.util.EncodingUtils.concatenate;
import static org.springframework.security.crypto.util.EncodingUtils.subArray;
@ -59,14 +61,6 @@ public abstract class AbstractPasswordEncoder implements PasswordEncoder {
* Constant time comparison to prevent against timing attacks.
*/
protected 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;
return MessageDigest.isEqual(expected, actual);
}
}

View File

@ -17,6 +17,8 @@ package org.springframework.security.crypto.password;
import org.springframework.security.crypto.codec.Utf8;
import java.security.MessageDigest;
/**
* Utility for constant time comparison to prevent against timing attacks.
*
@ -33,16 +35,8 @@ class PasswordEncoderUtils {
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;
int result = expectedLength == actualLength ? 0 : 1;
for (int i = 0; i < actualLength; i++) {
byte expectedByte = expectedLength <= 0 ? 0 : expectedBytes[i % expectedLength];
byte actualByte = actualBytes[i % actualLength];
result |= expectedByte ^ actualByte;
}
return result == 0;
return MessageDigest.isEqual(expectedBytes, actualBytes);
}
private static byte[] bytesUtf8(String s) {

View File

@ -16,6 +16,7 @@
package org.springframework.security.crypto.password;
import java.security.GeneralSecurityException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Base64;
@ -141,22 +142,7 @@ public class Pbkdf2PasswordEncoder implements PasswordEncoder {
public boolean matches(CharSequence rawPassword, String encodedPassword) {
byte[] digested = decode(encodedPassword);
byte[] salt = subArray(digested, 0, this.saltGenerator.getKeyLength());
return matches(digested, 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;
return MessageDigest.isEqual(digested, encode(rawPassword, salt));
}
private byte[] decode(String encodedBytes) {

View File

@ -23,6 +23,8 @@ import org.springframework.security.crypto.codec.Utf8;
import org.springframework.security.crypto.keygen.BytesKeyGenerator;
import org.springframework.security.crypto.keygen.KeyGenerators;
import java.security.MessageDigest;
/**
* This {@link PasswordEncoder} is provided for legacy purposes only and is not considered
* secure.
@ -79,7 +81,7 @@ public final class StandardPasswordEncoder implements PasswordEncoder {
public boolean matches(CharSequence rawPassword, String encodedPassword) {
byte[] digested = decode(encodedPassword);
byte[] salt = subArray(digested, 0, saltGenerator.getKeyLength());
return matches(digested, digest(rawPassword, salt));
return MessageDigest.isEqual(digested, digest(rawPassword, salt));
}
// internal helpers
@ -105,21 +107,6 @@ public final class StandardPasswordEncoder implements PasswordEncoder {
return Hex.decode(encodedPassword);
}
/**
* Constant time comparison to prevent against timing attacks.
*/
private 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 static final int DEFAULT_ITERATIONS = 1024;
}

View File

@ -15,6 +15,7 @@
*/
package org.springframework.security.crypto.scrypt;
import java.security.MessageDigest;
import java.util.Base64;
import org.apache.commons.logging.Log;
@ -152,15 +153,7 @@ public class SCryptPasswordEncoder implements PasswordEncoder {
byte[] generated = SCrypt.generate(Utf8.encode(rawPassword), salt, cpuCost, memoryCost, parallelization,
keyLength);
if (derived.length != generated.length) {
return false;
}
int result = 0;
for (int i = 0; i < derived.length; i++) {
result |= derived[i] ^ generated[i];
}
return result == 0;
return MessageDigest.isEqual(derived, generated);
}
private String digest(CharSequence rawPassword, byte[] salt) {

View File

@ -256,15 +256,8 @@ public class TokenBasedRememberMeServices extends AbstractRememberMeServices {
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;
return MessageDigest.isEqual(expectedBytes, actualBytes);
}
private static byte[] bytesUtf8(String s) {