Is this string comparison code resistant to timing attacks?

Question

I have to create an API that authenticates a request by checking if the token in the request is equal to the valid token stored in our database. Initially I used the == operator to compare the two tokens, but then I learned that this was vulnerable to a timing attack.

So I wrote the below code in C# to do the token comparison:

static bool constantTimeStringCompare(string a, string b)
        {
            if (a.Length != b.Length)
            {
                return false;
            }

            int result = 0;
            for (int i =0; i < a.Length; i++)
            {
                result |= a[i] ^ b[i];
            }
            if (result == 0)
            {
                return a == b;
            } else
            {
                return false;
            }
        }

Since I am paranoid about "rolling my own crypto" I added the check at the end of the function that does the naïve string comparison to double check that the tokens really are the same. Does doing this open me up to a timing attack? I don't think it does, as the naïve string comparison only gets run if the xor check thinks the two strings are the same, which will only happen if they really are the same or if there is a really rare bug in the code -- the upshot is that it will rarely get run, thus the running time of the function will be constant.

Personally I think it's a bad idea for me to be doing this (we should be using client certificates), but my company doesn't care.

Have you considered using [`CryptographicOperations.FixedTimeEquals`](https://docs.microsoft.com/en-us/dotnet/api/system.security.cryptography.cryptographicoperations.fixedtimeequals?view=netcore-2.1) instead of rolling your own function? It's available in .NET Core 2.1+ — rink.attendant.6, Jul 13 '21 at 19:16
I can't. I have to use .net frameowrk 4.6.7 because we are using ASMX (lol) But I did have a look at that code, and it does the same thing I am doing except they use subtraction and span... — Weare Mwam, Jul 15 '21 at 05:21

score 1 · Accepted Answer · answered Jul 10 '21 at 04:49

The code is clearly not constant time since

The first part does an immediate return if the length differ
The second part depends on the length of the strings, although it is constant time for a specific length
The additional comparison before return depends on the common prefix and the length of the string. Since the common prefix is exactly the string (since they are the same) this only adds another dependency on the string length

... vulnerable to a timing attack

Just because something is not constant time comparison does not mean that it can be used for a timing attack. It matters what it is used for. If a hash gets compared to another hash not having a constant time comparison does not matter - see Why should password hash verification be time constant? (Hint: there is no need) or Does bcrypt compare the hashes in “length-constant” time?.

Sorry, I know the first if statement is not constant time, but I don't care if the attacker finds out how long the token is. Anyway, I am comparing the raw tokens, but I think I should store the token's hash and then hash the incoming token, so that should fix this issue. Thanks — Weare Mwam, Jul 10 '21 at 05:57

Is this string comparison code resistant to timing attacks?

1 Answers1