.NET Core MVC - Future proof Hashing of passwords

Question

I have written a Helper for Hashing passwords in .NET Core.

It should have the following constraints:

• Future proof (It should be possible to upgrade the Hashing Engine to support Legacy and New Hashes)

• It should be secure

• It should be easy to use

I want to know from you, if I meet these constraints, especially if it is Future proof, and secure.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Security.Cryptography;
using JetBrains.Annotations;
using Microsoft.AspNetCore.Cryptography.KeyDerivation;
namespace MyNamespace
{
 /// <summary>
 /// Provides methods Hashing and Verification of clear texts
 /// </summary>
 [PublicAPI]
 public class HashingManager
 {
 /// <summary>
 /// The default number of Iterations
 /// </summary>
 private const int DefaultIterations = 10000;
 /// <summary>
 /// Provides Information about a specific Hash Version
 /// </summary>
 private class HashVersion
 {
 public short Version { get; set; }
 public int SaltSize { get; set; }
 public int HashSize { get; set; }
 public KeyDerivationPrf KeyDerivation { get; set; }
 }
 /// <summary>
 /// Holds all possible Hash Versions
 /// </summary>
 private readonly Dictionary<short, HashVersion> _versions = new Dictionary<short, HashVersion>
 {
 {
 1, new HashVersion
 {
 Version = 1,
 KeyDerivation = KeyDerivationPrf.HMACSHA512,
 HashSize = 256 / 8,
 SaltSize = 128 / 8
 }
 }
 };
 /// <summary>
 /// The default Hash Version, which should be used, if a new Hash is Created
 /// </summary>
 private HashVersion DefaultVersion => _versions[1];
 /// <summary>
 /// Checks if a given hash uses the latest version
 /// </summary>
 /// <param name="data">The hash</param>
 /// <returns>Is the hash of the latest version?</returns>
 public bool IsLatestHashVersion(byte[] data)
 {
 var version = BitConverter.ToInt16(data, 0);
 return version == DefaultVersion.Version;
 }
 /// <summary>
 /// Checks if a given hash uses the latest version
 /// </summary>
 /// <param name="data">The hash</param>
 /// <returns>Is the hash of the latest version?</returns>
 public bool IsLatestHashVersion(string data)
 {
 var dataBytes = Convert.FromBase64String(data);
 return IsLatestHashVersion(dataBytes);
 }
 /// <summary>
 /// Gets a random byte array
 /// </summary>
 /// <param name="length">The length of the byte array</param>
 /// <returns>The random byte array</returns>
 public byte[] GetRandomBytes(int length)
 {
 var data = new byte[length];
 using (var randomNumberGenerator = RandomNumberGenerator.Create())
 {
 randomNumberGenerator.GetBytes(data);
 }
 return data;
 }
 /// <summary>
 /// Creates a Hash of a clear text
 /// </summary>
 /// <param name="clearText">the clear text</param>
 /// <param name="iterations">the number of iteration the hash alogrythm should run</param>
 /// <returns>the Hash</returns>
 public byte[] Hash(string clearText, int iterations = DefaultIterations)
 {
 //get current version
 var currentVersion = DefaultVersion;
 //get the byte arrays of the hash and meta information
 var saltBytes = GetRandomBytes(currentVersion.SaltSize);
 var versionBytes = BitConverter.GetBytes(currentVersion.Version);
 var iterationBytes = BitConverter.GetBytes(iterations);
 var hashBytes = KeyDerivation.Pbkdf2(clearText, saltBytes, currentVersion.KeyDerivation, iterations, currentVersion.HashSize);
 //calculate the indexes for the combined hash
 var indexVersion = 0;
 var indexIteration = indexVersion + 2;
 var indexSalt = indexIteration + 4;
 var indexHash = indexSalt + currentVersion.SaltSize;
 //combine all data to one result hash
 var resultBytes = new byte[2 + 4 + currentVersion.SaltSize + currentVersion.HashSize];
 Array.Copy(versionBytes, 0, resultBytes, indexVersion, 2);
 Array.Copy(iterationBytes, 0, resultBytes, indexIteration, 4);
 Array.Copy(saltBytes, 0, resultBytes, indexSalt, currentVersion.SaltSize);
 Array.Copy(hashBytes, 0, resultBytes, indexHash, currentVersion.HashSize);
 return resultBytes;
 }
 /// <summary>
 /// Creates a Hash of a clear text and convert it to a Base64 String representation
 /// </summary>
 /// <param name="clearText">the clear text</param>
 /// <param name="iterations">the number of iteration the hash alogrythm should run</param>
 /// <returns>the Hash</returns>
 public string HashToString(string clearText, int iterations = DefaultIterations)
 {
 var data = Hash(clearText, iterations);
 return Convert.ToBase64String(data);
 }
 /// <summary>
 /// Verifies a given clear Text against a hash
 /// </summary>
 /// <param name="clearText">The clear text</param>
 /// <param name="data">The hash</param>
 /// <returns>Is the hash equal to the clear text?</returns>
 public bool Verify(string clearText, byte[] data)
 {
 //Get the current version and number of iterations
 var currentVersion = _versions[BitConverter.ToInt16(data, 0)];
 var iteration = BitConverter.ToInt32(data, 2);
 //Create the byte arrays for the salt and hash
 var saltBytes = new byte[currentVersion.SaltSize];
 var hashBytes = new byte[currentVersion.HashSize];
 //Calculate the indexes of the salt and the hash
 var indexSalt = 2 + 4; // Int16 (Version) and Int32 (Iteration)
 var indexHash = indexSalt + currentVersion.SaltSize;
 //Fill the byte arrays with salt and hash
 Array.Copy(data, indexSalt, saltBytes, 0, currentVersion.SaltSize);
 Array.Copy(data, indexHash, hashBytes, 0, currentVersion.HashSize);
 //Hash the current clearText with the parameters given via the data
 var verificationHashBytes = KeyDerivation.Pbkdf2(clearText, saltBytes, currentVersion.KeyDerivation, iteration, currentVersion.HashSize);
 //Check if generated hashes are equal
 return hashBytes.SequenceEqual(verificationHashBytes);
 }
 /// <summary>
 /// Verifies a given clear Text against a hash
 /// </summary>
 /// <param name="clearText">The clear text</param>
 /// <param name="data">The hash</param>
 /// <returns>Is the hash equal to the clear text?</returns>
 public bool Verify(string clearText, string data)
 {
 var dataBytes = Convert.FromBase64String(data);
 return Verify(clearText, dataBytes);
 }
 }
}

Usage:

var hashing = new HashingManager();
var hash = hashing.HashToString("test");
var isValid = hashing.Verify("test", hash);

Is there something which you would improve?

• Mast: You're asking about security. What's your threat model?
• lonix: Why did you choose those defaults for hash size and salt? Are they "standard" or just your preference?
• Christian Gollhardt: I am no security expert. They are just preferences I probably got after reading some blogs. I have no source of truth, sorry @Ionix

Answer 1

Usability

It should be easy to use

I consider this easy to use because all technical details are hidden from the end-user.

var hashing = new HashingManager();
var hash = hashing.HashToString("test");
var isValid = hashing.Verify("test", hash);

---

Security

It should be secure

You are using a well estblished algorithm Pbkdf2 together with a technique called key stretching with a high amount of iterations. This is considered secure.

Some would take it a step further and make the number of iterations a variable as well and store it along the digest. _{This would potentially require greater rainbow tables. A potential h3ck0r now requires combinations of passwords, salts ánd iterations to attack. The exact impact on entropy requires a citation...}

hashBytes.SequenceEqual(verificationHashBytes) is optimized to exit early, which is considered a possible weakness in the algorithm. Use SlowEquals instead.

---

Versioning

Future proof (It should be possible to upgrade the Hashing Engine to support Legacy and New Hashes)

Since your first two bytes represent the version, you are future proof. Suppose the entire structure of HashVersion would change (obsolete fields, new fields, ..) then you would still be able to read the first two bytes first and decide how to read the remaining data based on the version.

For instance, in pseudo code:

var version = Read<int>(raw, 0);
if (version <= NewHashVersionThreshold) 
{
 var hashVersionLegacy = Read<HashVersionLegacy>(raw, 2));
 // .. handle further
} else 
{
 var hashVersion = Read<HashVersion>(raw, 2));
 // .. handle further
}

---

I don't like this line:

private HashVersion DefaultVersion => _versions[1];

You'd have to change this code for each new version. I would return the latest version, which is the HashVersion with highest Version property.

---

This method seems of little use to me: IsLatestHashVersion. What if false is returned, what would you do with that information? I find it more useful to return how a hash version compares to the DefaultVersion. Suppose the software's DefaultVersion is 3 at some point and we verify a hash of version ..

Backward-compatibility enforced

• .. version 2: the software could verify the hash using the structure of version 2.

Forward-compatibility limited

• .. version 4: the software is unable to verify the hash.

• Christian Gollhardt: Thank you :) The idea of IsLatestHashVersion is, that the user (of the class) can simple rehash the data (and store the new hash in the persistance layer)
• Jins Peter: @ChristianGollhardt How could If I add a new version in the verision list.? What does the properties in the versions list stand for. If i intorduce a new version and set it up as a default version, will the Verify give true?
• Christian Gollhardt: The properties are just the alogrythm and the size. Verify currently takes automaticaly care about the hashversion, so yes (The version is stored in the hash). As per this advise, you probably want to limit Forward Compatibility or rehash when IsLatestHashVersion is false. @JinsPeter
• Jins Peter: Yeah. I'm fine with ReHashing.. But I dont understand when do I need to introduce a new Version
• Christian Gollhardt: First: it is the same, but just written with bits in mind (8 bits = 1 byte => 256 bit / 8 bit per byte = 32 byte). Second: Whenever Security Experts says so. Generaly speaking the longer it takes to calculate the hash (Iterations) the harder it is to bruteforce them. I am no security expert, hence I asked the question myself ;) @JinsPeter

Answer 2

Just a minor thing, using

const int sizeOfVersion = sizeof(Int16);
const int sizeOfIterationCount = sizeof(Int32);

would remove the "magic numbers" 2 and 4 and aid comprehension a little :)