Issue #1556 Timing attack

This commit is contained in:
Greg Wilkins 2017-08-18 11:38:55 +10:00
parent 643c3175a0
commit a7e8b4220a
2 changed files with 42 additions and 28 deletions

View File

@ -86,51 +86,47 @@ public abstract class Credential implements Serializable
}
/**
* <p>Utility method that replaces String.equals() to avoid timing attacks.</p>
* <p>Utility method that replaces String.equals() to avoid timing attacks.
* The length of the loop executed will always be the length of the unknown credential</p>
*
* @param s1 the first string to compare
* @param s2 the second string to compare
* @param known the first string to compare (should be known string)
* @param unknown the second string to compare (should be the unknown string)
* @return whether the two strings are equal
*/
protected static boolean stringEquals(String s1, String s2)
protected static boolean stringEquals(String known, String unknown)
{
if (s1 == s2)
if (known == unknown)
return true;
if (s1 == null || s2 == null)
if (known == null || unknown == null)
return false;
boolean result = true;
int l1 = s1.length();
int l2 = s2.length();
if (l1 != l2)
result = false;
int l = Math.min(l1, l2);
for (int i = 0; i < l; ++i)
result &= s1.charAt(i) == s2.charAt(i);
return result;
int l1 = known.length();
int l2 = unknown.length();
for (int i = 0; i < l2; ++i)
result &= known.charAt(i%l1) == unknown.charAt(i);
return result && l1 == l2;
}
/**
* <p>Utility method that replaces Arrays.equals() to avoid timing attacks.</p>
* <p>Utility method that replaces Arrays.equals() to avoid timing attacks.
* The length of the loop executed will always be the length of the unknown credential</p>
*
* @param b1 the first byte array to compare
* @param b2 the second byte array to compare
* @param known the first byte array to compare (should be known value)
* @param unknown the second byte array to compare (should be unknown value)
* @return whether the two byte arrays are equal
*/
protected static boolean byteEquals(byte[] b1, byte[] b2)
protected static boolean byteEquals(byte[] known, byte[] unknown)
{
if (b1 == b2)
if (known == unknown)
return true;
if (b1 == null || b2 == null)
if (known == null || unknown == null)
return false;
boolean result = true;
int l1 = b1.length;
int l2 = b2.length;
if (l1 != l2)
result = false;
int l = Math.min(l1, l2);
for (int i = 0; i < l; ++i)
result &= b1[i] == b2[i];
return result;
int l1 = known.length;
int l2 = unknown.length;
for (int i = 0; i < l2; ++i)
result &= known[i%l1] == unknown[i];
return result && l1 == l2;
}
/**

View File

@ -76,4 +76,22 @@ public class CredentialTest
assertTrue (p1.equals(p2));
}
@Test
public void testStringEquals()
{
assertTrue(Credential.stringEquals("foo","foo"));
assertFalse(Credential.stringEquals("foo","fooo"));
assertFalse(Credential.stringEquals("foo","fo"));
assertFalse(Credential.stringEquals("foo","bar"));
}
@Test
public void testBytesEquals()
{
assertTrue(Credential.byteEquals("foo".getBytes(),"foo".getBytes()));
assertFalse(Credential.byteEquals("foo".getBytes(),"fooo".getBytes()));
assertFalse(Credential.byteEquals("foo".getBytes(),"fo".getBytes()));
assertFalse(Credential.byteEquals("foo".getBytes(),"bar".getBytes()));
}
}