Parameterize System.Security.Cryptography.Rfc2898DeriveBytes for HMAC

[prajaybasu]

Currently System.Security.Cryptography.Rfc2898DeriveBytes is hardcoded to use HMACSHA1, it would be nice if HMAC could be parameterized. (More specifically, so that developers can use HMACSHA256 for more security.) Eg : public Rfc2898DeriveBytes(HMAC hmac, string password, byte[] salt, int iterations).

[bartonjs]

I thought I had an issue for this; but maybe it was only in the pre-git system. Anyways, I'm in favor.

[morganbr]

My reading of the RFC is that HMACSHA1 is the only algorithm it supports (though I could certainly be wrong). Maybe there are other KDFs that we could look into though.

[bartonjs]

I read the RFC as it being an open choice. It says the PRF is an option, and as "an example" they reference HMAC-SHA-1 (contrast it to PBKDF1 where they enumerate MD2, MD5, SHA-1 as the only options).

Plus, Windows seems to believe it's a choice, too: https://msdn.microsoft.com/en-us/library/windows/desktop/hh448516(v=vs.85).aspx says that naming your HMAC's hash algorithm is a required parameter for NCryptKeyDerivation / PBKDF2.

[prajaybasu]

@morganbr I know some popular implementations which use PBKDF2 with HMACSHA256 (see Authy, and Scrypt ).

I am quite new to C# and .NETCore in general (so I might not make sense sometimes) but - System.Security.Cryptography has quite a few different implementations and might drive some people to make their own "consistent" implementations. (FIPS and X-Plat make stuff a bit different for this package, I think ?) :

• Now, for example, I believe .NETCore.UniversalWindowsPlatform implements something called KeyDerivationAlgorithmProvider and you would implement PBKDF2 in UWP with your desired Algorithm like this. (Again I am new to .NETCore arch, but could this have been implemented in System.Cryptography ?; Because as far as my knowledge goes, Encryption APIs should be portable (unless it is some UWP specific feature like Windows Hello))
• while System.Security.Cryptography only supports PBKDF2's generic RFC implementation with only SHA1.
• And then there is this cross platform library which wraps up stuff and promises "consistent" APIs.

It would be better if it would be implemented as something like PBKDF2 : KeyDerivationAlgorithm or Scrypt : KeyDerivationAlgorithm ; i.e, make KeyDerivationAlgorithm a primitive like HashAlgorithm, because algorithms like Scrypt are also KeyDerivationAlgorithm(s) like PBKDF2 - or one might want to make their own implementation of KDF from something like bcrypt to generate keys (since bcrypt has 192 bit keys only, I believe) and not want to completely make a new implementation by themselves (which kind of decreases maintainability).

[blowdart]

This would enable me to cull some asp.net code, as we have our own hasher now for PBKDF2 which uses SHA256 underneath in identity.

[bartonjs]

Proposal

Add a HashAlgorithmName parameter to each of the overloads for Rfc2898DeriveBytes constructors which take an iteration count, and add a property allowing the configured algorithm to be queried.

    public partial class Rfc2898DeriveBytes : System.Security.Cryptography.DeriveBytes
    {
        public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations) { }
+       public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations, HashAlgorithmName hashAlgorithm) { }
        public Rfc2898DeriveBytes(string password, byte[] salt) { }
        public Rfc2898DeriveBytes(string password, byte[] salt, int iterations) { }
+       public Rfc2898DeriveBytes(string password, byte[] salt, int iterations, HashAlgorithmName hashAlgorithm) { }
        public Rfc2898DeriveBytes(string password, int saltSize) { }
        public Rfc2898DeriveBytes(string password, int saltSize, int iterations) { }
+       public Rfc2898DeriveBytes(string password, int saltSize, int iterations, HashAlgorithmName hashAlgorithm) { }
+       public HashAlgorithmName HashAlgorithm { get { throw null; } }
        public int IterationCount { get { return default(int); } set { } }
        public byte[] Salt { get { return default(byte[]); } set { } }
        protected override void Dispose(bool disposing) { }
        public override byte[] GetBytes(int cb) { return default(byte[]); }
        public override void Reset() { }
}

The reason only the "long" constructors are gaining the new parameter is to indicate that the iteration count is an important choice for this object. When being used to validate user logons, for example, the iteration count and hash algorithm should be serialized for object recovery. Over time a system should be increasing the iteration count and/or upgrading the hash algorithm, which can be done upon validating the existing credentials.

Notes to implementors

Test vectors for SHA-2-256, SHA-2-384, and SHA-2-512 should be provided. Between 2 and 5 each. These test cases should come from (in decreasing order of preference):

• An RFC
• A NIST verification sample
• An implementation verification test (OpenSSL, NaCl, etc)
• Something made up.

And, of course, provide a citation for where they originated.

It may also make sense at this time to replace the managed implementation with native calls, PKCS5_PBKDF2_HMAC via OpenSSL (Unix) and NCryptKeyDeriviation (Windows); but that is not a requirement of this change.

[morganbr]

I generally like allowing users to specify a HashAlgorithmName. However, I don't think specifying an algorithm should be directly tied to having to specify iteration count -- most people would probably just like to have a default chosen for them. I'd add that we're having a bit of a combinatorial expansion of constructors here. Would it make sense to just make the HashAlgorithm property settable?

[weshaggard]

We've reviewed the proposal and it seems fine.

[prajaybasu]

Can't there be any better name than DeriveBytes and Rfc2898DeriveBytes ? Maybe make it consistent with UWP ?