31

In the asp.net core PasswordHasher type there is is remark on the VerifyHashedPassword method

 /// <remarks>Implementations of this method should be time consistent.</remarks>

And then to compare the hashes it uses code that is deliberately not optimised and written not do early exits in the loop.

// Compares two byte arrays for equality. The method is specifically written so that the loop is not optimized.
[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
private static bool ByteArraysEqual(byte[] a, byte[] b)
{
    if (a == null && b == null)
    {
        return true;
    }
    if (a == null || b == null || a.Length != b.Length)
    {
        return false;
    }
    var areSame = true;
    for (var i = 0; i < a.Length; i++)
    {
       areSame &= (a[i] == b[i]);
    }
    return areSame;
}

At first I thought that without this timing could be used to determine how close the hash was, if it takes longer then more of the hash is the same.

However this doesn't make sense because the hash has gone through 1000 iterations of SHA256 at this point. So any change in the password would produce a completely different hash, and knowing that your password produces almost the correct hash does not help you find the correct one.

What is the purpose of ensuring a constant time hash verification?

forest
  • 66,706
  • 20
  • 212
  • 270
trampster
  • 391
  • 3
  • 6
  • 3
    Is that function used for anything other than comparing hashes? – forest May 09 '19 at 02:12
  • no it is only used for comparing hashes – trampster May 09 '19 at 02:14
  • 1
    On a side(-attack) note this code assumes that byte comparisons are constant time which isn't guaranteed. It's good that it probably doesn't matter. – JimmyJames May 09 '19 at 16:06
  • 1
    For better (or worse), code gets copied around. In the current AspNetCore repo BinaryBlob there is a near-identical method that can be used to compare any byte[]. Just because the code you write isn't used for something right now doesn't mean it won't be misused later! – Carl Walsh May 09 '19 at 23:35
  • 1
    You can use varName |= a[i] ^ b[i]; in the loop instead. Initialize the variable to zero. Finally, return varName == 0. The XOR of two values is zero if and only if the values are the same. Once you OR a non-zero value into varName, the set bits of the value |='d into varName will stay set. Bitwise operations on values of the native machine word size are constant time, so the modified function will be constant time too. (Assuming the compiler doesn't, as an optimization, insert an early exit into the loop.) – Future Security May 10 '19 at 15:40
  • Is there anything else you'd like me to add to my answer? – forest Jan 21 '21 at 06:18

1 Answers1

44

Assuming neither of the hashes are secret and the hashes are secure (which SHA-256 is), there is no reason to check the hash in constant time. In fact, comparing hashes is one of the well-known alternatives to verifying passwords within a constant time routine. I can't say what reason the developers would give for doing this, but it is not technically necessary to make it constant time. Most likely, they were just being cautious. Non-constant time code in a cryptographic library makes auditors anxious.

More information about the theoretical weaknesses is discussed in an answer on the Cryptography site. It explains how, with a significant amount of queries, it can be possible to discover the first several bytes of the hash, which makes it possible to perform an offline computation to discard candidate passwords that obviously wouldn't match (their hash doesn't match the first few discovered bytes of the real hash) and avoid sending them to the password checking service, and why this is unlikely to be a real issue.

forest
  • 66,706
  • 20
  • 212
  • 270
  • 37
    "Non-constant time code in a cryptographic library makes auditors anxious." - this! If the code is constant time, nobody has to fret about that side channel. If it not, you have to write a comment (or design note) explaining why it's not a problem. – Martin Bonner supports Monica May 09 '19 at 09:18