[API Proposal]: Make digest size constants public

[vcsjones]

Background and motivation

For HMAC and hash classes, we have private constant sizes of the mac or digest size. I propose we make these public. We have various constants like 160, 20, 384, etc sprinkled throughout the libraries projects. It would be nice to use appropriately named constants in those places.

Currently the HashSize property exposes this, but it requires instantiating the hash object. This is less useful where static one-shots are preferred to be used.

API Proposal

namespace System.Security.Cryptography
{
     public class HMACMD5 : HMAC
     {
-        private const int HmacSizeBits = 128;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeInBits = 128;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public class HMACSHA1 : HMAC
     {
-        private const int HmacSizeBits = 160;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeInBits = 160;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public class HMACSHA256 : HMAC
     {
-        private const int HmacSizeBits = 256;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeInBits = 256;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public class HMACSHA384 : HMAC
     {
-        private const int HmacSizeBits = 384;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeInBits = 384;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public class HMACSHA512 : HMAC
     {
-        private const int HmacSizeBits = 512;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeInBits = 512;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public abstract class MD5 : HashAlgorithm
     {
-        private const int HashSizeBits = 128;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeInBits = 128;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public abstract class SHA1 : HashAlgorithm
     {
-        private const int HashSizeBits = 160;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeInBits = 160;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public abstract class SHA256 : HashAlgorithm
     {
-        private const int HashSizeBits = 256;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeInBits = 256;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public abstract class SHA384 : HashAlgorithm
     {
-        private const int HashSizeBits = 384;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeInBits = 384;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public abstract class SHA512 : HashAlgorithm
     {
-        private const int HashSizeBits = 512;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeInBits = 512;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

API Usage

Span<byte> buffer = stackalloc byte[SHA256.HashSizeInBytes];
SHA256.HashData(data, buffer);

Alternative Designs

As suggested below, we could consider using static properties, e.g.

public abstract class SHA256
{
    public static int HashSizeInBytes { get; } = 256;
    public static int HashSizeInBits { get; } = 256 / 8;
}

The JIT is clever enough to read through properties like this, so the performance should be similar, as I understand it (though I defer to the experts here). A hypothetical benefit here is it would fit with proposals like static interface members such as those proposed in #58882.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

For HMAC and hash classes, we have private constant sizes of the mac or digest size. I propose we make these public. We have various constants like 160, 20, 384, etc sprinkled throughout the libraries projects.

Currently the HashSize property exposes this, but it requires instantiating the hash object. This is less useful where static one-shots are preferred to be used.

API Proposal

namespace System.Security.Cryptography
{
     public class HMACMD5 : HMAC
     {
-        private const int HmacSizeBits = 128;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeBits = 128;
+        public const int HashSizeBytes = HmacSizeBits / 8;
     }

     public class HMACSHA1 : HMAC
     {
-        private const int HmacSizeBits = 160;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeBits = 160;
+        public const int HashSizeBytes = HmacSizeBits / 8;
     }

     public class HMACSHA256 : HMAC
     {
-        private const int HmacSizeBits = 256;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeBits = 256;
+        public const int HashSizeBytes = HmacSizeBits / 8;
     }

     public class HMACSHA384 : HMAC
     {
-        private const int HmacSizeBits = 384;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeBits = 384;
+        public const int HashSizeBytes = HmacSizeBits / 8;
     }

     public class HMACSHA512 : HMAC
     {
-        private const int HmacSizeBits = 512;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeBits = 512;
+        public const int HashSizeBytes = HmacSizeBits / 8;
     }

     public abstract class MD5 : HashAlgorithm
     {
-        private const int HashSizeBits = 128;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeBits = 128;
+        public const int HashSizeBytes = HashSizeBits / 8;
     }

     public abstract class SHA1 : HashAlgorithm
     {
-        private const int HashSizeBits = 160;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeBits = 160;
+        public const int HashSizeBytes = HashSizeBits / 8;
     }

     public abstract class SHA256 : HashAlgorithm
     {
-        private const int HashSizeBits = 256;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeBits = 256;
+        public const int HashSizeBytes = HashSizeBits / 8;
     }

     public abstract class SHA384 : HashAlgorithm
     {
-        private const int HashSizeBits = 384;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeBits = 384;
+        public const int HashSizeBytes = HashSizeBits / 8;
     }

     public abstract class SHA512 : HashAlgorithm
     {
-        private const int HashSizeBits = 512;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeBits = 512;
+        public const int HashSizeBytes = HashSizeBits / 8;
     }

API Usage

int digestSize = SHA256.HashSizeBits;

Alternative Designs

No response

Risks

No response

Author:	vcsjones
Assignees:	-
Labels:	api-suggestion, area-System.Security, untriaged
Milestone:	-

[vcsjones]

For consideration, I went with const instead of static readonly because there is no reality in which these values will change. SHA256 will always be 256 bits (32 bytes), so it’s fine if the compiler burns these values in at call sites. The advantage to const is these values can be used in switch statement cases, xunit inline theories, etc.

[Sergio0694]

I found myself hardcoding these as well, and agree that making those constants public makes perfect sense. Just a small naming question: wouldn't it be better to name them HashSizeInBits and HashSizeInBytes instead? I feel like it would technically be more correct than just "bits" and "bytes"? 🤔

[vcsjones]

Just a small naming question

Yes. I've even left that same feedback on other API proposals to include "In". Will fix in proposal. Thanks!

[Sergio0694]

Random question, this one is not an actual suggestion but just me thinking out loud. Would it make sense to make these static readonly properties instead of constants? The rationale being, if we ever wanted to have some/all of these types implement some interface with static abstract members in the future, which would also have those two members for the digest size, having them being static properties would allow them to directly implement the interface, instead of types having to then both expose those constants for backcompat, as well as explicit interface implementations for those interface members? I know this is not planned for now, but with all the shared static API surface growing among these types, was wondering whether making these new members static properties would give us more flexibility a few years down the line if we were to look into doing something like this. And for the time being, a static property just returning a constant would still be inlined by the JIT anyway, so codegen at call sites should presumably be the same too.

Just a thought 😄

[vcsjones]

if we ever wanted to have some/all of these types implement some interface with static abstract members in the future

There was some discussion of that in #58882. The idea is interesting but there are a number of concerns about it that make me think it's unlikely to happen. Among them, adding things to interfaces, even static members as I understand it, is a breaking change. Had .NET 6 shipped with static interface members, it would be challenging to further the design with proposals like #62489.

I'll leave it up to the API review board folks to weigh in on that, but I'll make note of it in the alternative designs section.

[bartonjs]

If we add them as constants, and then invent an IHashAlgorithm static interface later, we could just use explicit wrapping, like the numeric types do for IMinMax:

runtime/src/libraries/System.Private.CoreLib/src/System/Double.cs

Lines 861 to 869 in 312c66f

	//
	// IMinMaxValue
	//
	[RequiresPreviewFeatures(Number.PreviewFeatureMessage, Url = Number.PreviewFeatureUrl)]
	static double IMinMaxValue<double>.MinValue => MinValue;
	[RequiresPreviewFeatures(Number.PreviewFeatureMessage, Url = Number.PreviewFeatureUrl)]
	static double IMinMaxValue<double>.MaxValue => MaxValue;

The use of const (vs a get-only property) means it can be used in more contexts, like the size of a fixed inline array (or the computation of some other const). And it really is a constant... SHA256 would be a different algorithm if one day we thought we needed to change HashSizeInBits to be 258.

[bartonjs]

Video

Looks good as proposed

namespace System.Security.Cryptography
{
     public class HMACMD5 : HMAC
     {
-        private const int HmacSizeBits = 128;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeInBits = 128;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public class HMACSHA1 : HMAC
     {
-        private const int HmacSizeBits = 160;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeInBits = 160;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public class HMACSHA256 : HMAC
     {
-        private const int HmacSizeBits = 256;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeInBits = 256;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public class HMACSHA384 : HMAC
     {
-        private const int HmacSizeBits = 384;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeInBits = 384;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public class HMACSHA512 : HMAC
     {
-        private const int HmacSizeBits = 512;
-        private const int HmacSizeBytes = HmacSizeBits / 8;
+        public const int HashSizeInBits = 512;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public abstract class MD5 : HashAlgorithm
     {
-        private const int HashSizeBits = 128;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeInBits = 128;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public abstract class SHA1 : HashAlgorithm
     {
-        private const int HashSizeBits = 160;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeInBits = 160;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public abstract class SHA256 : HashAlgorithm
     {
-        private const int HashSizeBits = 256;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeInBits = 256;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public abstract class SHA384 : HashAlgorithm
     {
-        private const int HashSizeBits = 384;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeInBits = 384;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }

     public abstract class SHA512 : HashAlgorithm
     {
-        private const int HashSizeBits = 512;
-        private const int HashSizeBytes = HashSizeBits / 8;
+        public const int HashSizeInBits = 512;
+        public const int HashSizeInBytes = HashSizeInBits / 8;
     }
}

[deeprobin]

This could be labeled "easy" for new contributors imo.