Runtime: Add time-constant equals and other utility methods to System.Security

[clintharrison]

In applications sensitive to side-channel timing attacks, a constant time equality check can be used instead of a normal short-circuiting String.Equals (or similar for byte[], etc.).

This is functionality often duplicated by application developers, copied and pasted from StackOverflow and what not. This is easy to subtly get wrong, and probably belongs somewhere in System.Security.

We can see an example of this already implemented in an internal .NET class: Microsoft.AspNetCore.Cryptography.Internal, but this isn't really the best place for it 😃

---

Edited by @bartonjs to match what was approved:

namespace System.Security.Cryptography
{
    // new type in System.Security.Cryptography.Primitives.dll
    public static class CryptographicOperations
    {
        public static bool FixedTimeEquals(ReadOnlySpan<byte> left, ReadOnlySpan<byte> right);
        public static void ZeroMemory(Span<byte> span);
    }

    // Existing type in System.Security.Cryptography.Algorithms.dll
    public abstract class RandomNumberGenerator
    {
        public static void Fill(Span<byte> data);
    }
}

[bartonjs]

We have the code, we just need to pick a class/namespace and a release vehicle. Right now new API in 1.1 is difficult; so I'll call this 1.2 (and on a parallel endeavor net463).

[kevinchalet]

@bartonjs if such a comparer is introduced, I suppose it won't be available for <.NET 4.6.3 and <.NET Standard 1.7/8?

[bartonjs]

@PinpointTownes Correct; as far as I know we have no notion of a back-in-time API addition. Unless it's just added as a completely separate package, I guess.

[kevinchalet]

Thanks!

Unless it's just added as a completely separate package, I guess.

Could that be an acceptable option? That's the approach @davidfowl seems to prefer: https://github.com/aspnet/Common/issues/140#issuecomment-241476590

[karelz]

We need formal API proposal

[kevinchalet]

If I had to design such an API, I'd personally go with something like that:

public class CryptographicComparer : IEqualityComparer<byte[]>
{
    public static CryptographicComparer Default { get; }

    public bool Equals(byte[] x, byte[] y);
    public int GetHashCode(byte[] obj);
}

It could even support string comparison:

public class CryptographicComparer : IEqualityComparer<byte[]>, IEqualityComparer<string>
{
    public static CryptographicComparer Default { get; }

    public bool Equals(byte[] x, byte[] y);
    public bool Equals(string x, string y);
    public int GetHashCode(byte[] obj);
    public int GetHashCode(string obj);
}

[GrabYourPitchforks]

I don't know if it makes sense for this to be exposed as an IEqualityComparer. That interface is intended to be used in collections like sets and dictionaries, which is why it also exposes _GetHashCode_ in addition to _Equals_, whereas the typical use case for time-constant comparison is to compare two values in isolation without the involvement of any collection.

My ideal API would be just a single static method bool TimeConstantEquals(byte[] a, byte[] b), perhaps also with Span<byte>-based overloads for convenience. Anybody who wants to write an IEqualityComparer on top of this is welcome to; they'd just have to choose an appropriate time-constant hash code, which isn't quite as trivial as it sounds. If it ends up being too problematic for devs to write themselves and if there's a valid scenario for it then we can consider adding an IEqualityComparer in-box in the future.

[GrabYourPitchforks]

Concrete proposal using the existing System.Buffers class.

namespace System {
    public static class Buffer {
        // Compares two buffers for equality; short-circuiting
        public static bool AreEqual(Span<byte> a, Span<byte> b);

        // Compares two buffers for equality; time-constant
        public static bool AreEqualTimeConstant(Span<byte> a, Span<byte> b);
    }
}

Behavior: the time-constant implementation is allowed to short-circuit and return _false_ iff _a_ and _b_ are different lengths. It can't short-circuit for any other reason, including when _a_ and _b_ point to the same memory address.

[GrabYourPitchforks]

Alternatively, we could introduce a new type CryptoUtilities which could serve as a dumping ground for crypto helper APIs. A proposal (including some additional APIs) is below.

namespace System.Security.Cryptography {
    public static class CryptoUtilities {
        // Essentially checks that bits is a non-negative multiple of 8, then divides by 8
        public static int BitsToBytes(int bits);

        // Checks that bytes is non-negative, then multiplies by 8 w/ overflow checking
        public static int BytesToBits(int bytes);

        // Basically wraps RNGCryptoServiceProvider, but more convenient
        public static void FillRandom(Span<byte> span);

        // Compares two buffers for equality; time-constant
        public static bool TimeConstantEquals(Span<byte> a, Span<byte> b);

        // Securely zeroes out the buffer in a manner that won't be optimized away
        public static void ZeroMemory(Span<byte> span);
    }
}

[bartonjs]

public static class CryptographicOperations, maybe? With FixedTimeEquals (since it varies on a.Length it's not "constant", per some people's complaints). BitsToBytes should probably be (bits + 7) >> 3, or, at least, one of those would be required :)

[GrabYourPitchforks]

Sure, those names seem sound to me. For your other suggestion, perhaps BitsToBytesRoundUp?

[GrabYourPitchforks]

(I've moved the proposal up to the very first comment.)

[nil4]

Could the parameters of FixedTimeEquals be declared ReadOnlySpan instead of Span, as the method is not mutating?

[GrabYourPitchforks]

@nil4 Great suggestion, I'll get this updated.

[karelz]

API review:

• Drop BitsToBytes, etc. as non-essential
• Change arg names of public static bool FixedTimeEquals(ReadOnlySpan<byte> x, ReadOnlySpan<byte> y); to left and right
• Rename ZeroMemory to Clear/TBD
• Move FillRandom to RandomNumberGenerator.

[bartonjs]

Update from feedback:

```C#
namespace System.Security.Cryptography
{
// new type in System.Security.Cryptography.Primitives.dll
public static class CryptographicOperations
{
public static bool FixedTimeEquals(ReadOnlySpan left, ReadOnlySpan right);
public static void ZeroMemory(Span span);
}

// Existing type in System.Security.Cryptography.Algorithms.dll
public abstract class RandomNumberGenerator
{
    // Parameter matches name for GetBytes methods.
    public static void Fill(Span<byte> data);
}

}
```

[terrajobst]

Video

Looks good as updated in the last comment.