Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

Create hash generator with configurable hash algorithm. Also need method to create salt. Implemented in visual studio 2015. Must work in DNX Core 5.0.

 public class HashGenerator : IHashGenerator
    {
        // Algorithm has to inherit from HashAlgorithm base class.
        // For example, another algorithm that can be used is: "System.Security.Cryptography.MD5";
        private const string HashAlgorithmName = "System.Security.Cryptography.SHA256";

        public string GenerateHash(string input, string salt)
        {
            var saltedPassword = Encoding.UTF8.GetBytes($"{input}{salt}");

            var hashAlgorithm = GetHashAlgorithm();
            var hashedPassword = hashAlgorithm.ComputeHash(saltedPassword);

            return Convert.ToBase64String(hashedPassword);
        }

        public string CreateSalt()
        {
            var buffer = new byte[25];
            using (var rng = RandomNumberGenerator.Create())
            {
                rng.GetBytes(buffer);
            }
            return Convert.ToBase64String(buffer);
        }

        private HashAlgorithm GetHashAlgorithm()
        {
            var typeInfo = Type.GetType(HashAlgorithmName).GetTypeInfo();
            var createMethod = typeInfo.GetDeclaredMethods("Create").First(f => f.GetParameters().Count() == 0);
            var hashAlgorithm = (HashAlgorithm)createMethod.Invoke(null, null);
            return hashAlgorithm;
        }
    }
share|improve this question

Right now you're stuck with the use of a constant. GetHashAlgorithm should receive, as a parameter, a Type. You then have 3 options for method signature :

The first one has the advantage of being sure that the Type inherits HashAlgorithm.

string GetHashAlgorithm<T>() where T : HashAlgorithm
string GetHashAlgorithm(Type hashAlgorithmType)
string GetHashAlgorithm(string typeName)

You shouldn't use a constant, as it is... constant. You then need to recompile your code to change the algorithm, which isn't good.

You could retrieve this string from your app.config file, which means you could configure it without recompiling.

If you don't mind being stuck with constant algorithm, I still don't think you should const them. Retrieve the HashAlgorithm from a factory maybe :

public class HashAlgorithmFactory
{
    public HashAlgorithm Create(/*Parameters to decide which algorithm maybe?*/)
    {
        //Conditions to decide which algorithm to return according to parameter
        return SHA256.Create();
    }
}

Also, is there a reason why you don't use your method CreateSalt in your CreateHash method instead of providing a salt parameter? I think it would make sense, but I don't have full knowledge of your needs.

share|improve this answer
1  
The CreateSalt method is created outside most likely because you would save the salt with the hashed password, so that you can reverify the password against any input with that salt. If the salt were not returned, or not allowed to be specified, then it would leave the issue that you could never verify a new password against the current hash. – EBrown Sep 28 '15 at 18:36

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.