I am writing a function that does a constant time compare of two byte arrays. I want it to be constant time to insure there is no side channel attacks possible.
- Does the code protect against timing attacks or other side channel attacks (of which I could fix)
- Any other suggestions for improvements
Note: This code is called infrequently so performance is not a big deal
/// <summary>
/// Performs a comparison of two identical sized byte arrays.
/// Performs in constant time to prevent timing/side channel attacks
/// </summary>
public int CompareConstantTime(byte[] bytesA, byte[] bytesB)
{
if (bytesA == null)
throw new ArgumentNullException(nameof(bytesA));
if (bytesB == null)
throw new ArgumentNullException(nameof(bytesB));
if (bytesA.Length != bytesB.Length)
throw new ArgumentOutOfRangeException("byte length must be equal");
//only the lest significant bit is used
int aIsLarger = 0;
int bIsLarger = 0;
unchecked
{
for (int i = 0; i < bytesA.Length; i++)
{
int byteA = bytesA[i];
int byteB = bytesB[i];
int byteAIsLarger = ((byteB - byteA) >> 8) & 1;
int byteBIsLarger = ((byteA - byteB) >> 8) & 1;
aIsLarger = aIsLarger | (byteAIsLarger & ~bIsLarger);
bIsLarger = bIsLarger | (byteBIsLarger & ~aIsLarger);
}
//fixes to standard compaire results (0 if A = B, 1 if A > B, -1: B > A)
int result = aIsLarger - bIsLarger;
return result;
}
}
1 Answer 1
Some nitpicking:
/// <summary> /// Performs a comparison of two identical sized byte arrays. /// Performs in constant time to prevent timing/side channel attacks /// </summary>
This does not answer arguably the main question one would have: what does it mean for one array to be "larger" than another, when both have equal size? Do you compare sums of the elements? Do you compare first non-equal pair of elements (looks like this one is correct)? Do you use some other even less obvious criteria? You should explain this in above documentation, so it is not necessary to read through obscure boolean logic in order to get the answer.
public int CompareConstantTime(byte[] bytesA, byte[] bytesB)
This method should probably be refactored into static extension method, so it is easier to re-use if needed.
public static int ConstantTimeCompareTo(this byte[] bytes, byte[] otherBytes)
throw new ArgumentOutOfRangeException("byte length must be equal");
There is no range per se at this point. I think base ArgumentException is a better fit.
O(N). He is talking about algorithm called constant time string/array comparison. See: security.stackexchange.com/questions/77428/… or crypto.stackexchange.com/questions/30958/… \$\endgroup\$int byteA = bytes[i] & 0xFFto get rid of the first sign extension. The deliberate underflow to negative values in the integer based calculation is probably all right, as internally they will just be registers - although setting the carry bit might still leak the tiniest bit of info. \$\endgroup\$