Why does my password checker not work? c#

Question

I`m trying to make a log in system for my app and want to use sha256 with salt for storing passwords. Problem is it when I hash the same password with the same salt again in order to check it I get different results. here is the code for both of the functions


 String[] securePassword(String password)
        {
            String[] result = new string[2];
            byte[] salt = new byte[32];
            System.Security.Cryptography.RNGCryptoServiceProvider.Create().GetBytes(salt);
            byte[] plainTextBytes =  UnicodeEncoding.Unicode.GetBytes(password);
          
            byte[] combinedBytes = new byte[plainTextBytes.Length + salt.Length];
            System.Buffer.BlockCopy(plainTextBytes, 0, combinedBytes, 0, plainTextBytes.Length);
            System.Buffer.BlockCopy(salt, 0, combinedBytes, plainTextBytes.Length, salt.Length);
            System.Security.Cryptography.HashAlgorithm hashAlgo = new System.Security.Cryptography.SHA256Managed();
            byte[] hash = hashAlgo.ComputeHash(combinedBytes);

            result[0] = Convert.ToBase64String(hash);
            result[1] = Convert.ToBase64String(salt);

            return result;
        }


bool check_password(String password_introduced,String Password,String Salt)
        {
            byte[] salt = Convert.FromBase64String(Salt);
            byte[] plainTextBytes = UnicodeEncoding.Unicode.GetBytes(password_introduced);

            byte[] combinedBytes = new byte[plainTextBytes.Length + salt.Length];
            System.Buffer.BlockCopy(plainTextBytes, 0, combinedBytes, 0, plainTextBytes.Length);
            System.Buffer.BlockCopy(salt, 0, combinedBytes, plainTextBytes.Length, salt.Length);
            System.Security.Cryptography.HashAlgorithm hashAlgo = new System.Security.Cryptography.SHA256Managed();
            byte[] hash = hashAlgo.ComputeHash(combinedBytes);
            String result = Convert.ToBase64String(hash);
            return (result == Password);
            
        }

I'm having a hard time following your logic with these two methods. Typically if I want to do something like this, I'll have a single method that turns the password into a hash. That method will take two parameters: `string password` and `string salt`. The first time I call it is when creating the account. I then take the result and store it in the database. Then when the user tries to log in again, I call the same method and see if the result matches what's in the database. — Casey Crookston, Dec 15 '20 at 20:55
(Continued)... the problem I see with this approach you are taking is... How do you preserve the salt? When the user comes back after a long absence, how do you know what salt to use? — Casey Crookston, Dec 15 '20 at 21:00
Ok, So when i create the password i call the first function and as you can see i return a vector of 2 strings, the first is the hash of the salted password and the second one is the salt, both of them are kept in a database in diffrent columns. When I try to check if a user used the right password i call the second function, the first parameter is the password introduced by the user, the second is the hash of the salted password and the third is the salt. The last two parameters come from the database. They were put there when the user created the account — Ciprian Florin, Dec 15 '20 at 21:05
Understood. Seems overly complex to me! But lemme see if I can spot the problem. — Casey Crookston, Dec 15 '20 at 21:10
I cannot reproduce your problem; works fine for me: `var hashAndSalt = securePassword("ABC"); var ok = check_password("ABC", hashAndSalt[0], hashAndSalt[1]);` returns true. — Klaus Gütter, Dec 16 '20 at 05:00
I found out what the problem was. Apparently those functions work just fine, but what happened is that i called the first function twice by mistake in order to get the 2 results instead of calling it once and storing both of them. That messes up the result becouse it generates 2 random hashes. Thank you all for your feedback though! — Ciprian Florin, Dec 16 '20 at 06:58

score 2 · Accepted Answer · answered Dec 15 '20 at 21:21

Well, what you have is working for me. I cleaned things up a little, including taking out repeating code (DRY) and standardizing the casing on your methods and parameters.

using System.Text;
using System;
using System.Security.Cryptography;

namespace ConsoleApp1
{
    public class Program
    {
        static void Main()
        {
            string[] result = SecurePassword("foobar");
            bool valid = CheckPassword("foobar", result[0], result[1]);
        }

        static string[] SecurePassword(string password)
        {
            string[] result = new string[2];

            byte[] salt = new byte[32];
            RNGCryptoServiceProvider.Create().GetBytes(salt);
            byte[] plainTextBytes = UnicodeEncoding.Unicode.GetBytes(password);

            byte[] hash = CreateHash(plainTextBytes, salt);

            result[0] = Convert.ToBase64String(hash);
            result[1] = Convert.ToBase64String(salt);

            return result;
        }


        static bool CheckPassword(string password_introduced, string password, string originalSalt)
        {
            byte[] salt = Convert.FromBase64String(originalSalt);
            byte[] plainTextBytes = UnicodeEncoding.Unicode.GetBytes(password_introduced);

            byte[] hash = CreateHash(plainTextBytes, salt);

            string result = Convert.ToBase64String(hash);
            return (result == password);

        }

        static byte[] CreateHash(byte[] plainTextBytes, byte[] salt)
        {
            byte[] combinedBytes = new byte[plainTextBytes.Length + salt.Length];
            Buffer.BlockCopy(plainTextBytes, 0, combinedBytes, 0, plainTextBytes.Length);
            Buffer.BlockCopy(salt, 0, combinedBytes, plainTextBytes.Length, salt.Length);
            HashAlgorithm hashAlgo = new System.Security.Cryptography.SHA256Managed();
            byte[] hash = hashAlgo.ComputeHash(combinedBytes);
            return hash;
        }
    }
}

The call to CheckPassword comes back true.

score 2 · Answer 2 · answered Dec 16 '20 at 08:15

2

Please use a dedicated password hash function like BCrypt instead. The SHA* hashes are not appropriate to hash passwords, because they are way too fast and therefore can be brute-forced too easily, even with an off the shelf GPU (about 7 Giga SHA256 per sec nowadays).

// Hash a new password for storing in the database.
// The function automatically generates a cryptographically safe salt.
string passwordHash =  BCrypt.HashPassword("my password");

// Check if the hash of the entered login password, matches the stored hash.
// The salt and the cost factor will be extracted from $existingHashFromDb.
bool isCorrect = BCrypt.Verify("my password", passwordHash);

answered Dec 16 '20 at 08:15

martinstoeckli

23,430
6
56
87

@Ciprian Florin, I do think this is a better approach than what you are using. In my answer, all I did was help solve your immediate problem. But overall, this is a safer approach. One of the security concerns with your original approach is that you are storing the salts in the database. If access to the db is ever compromised, your passwords are now very hackable. – Casey Crookston Dec 16 '20 at 16:45
1

@CaseyCrookston - Storing the salt in the database is actually safe and even necessary, the BCrypt hash will produce a string containing the salt, see this [answer](https://stackoverflow.com/a/20399775/575765) for details. It's really the immense speed which is the problem. – martinstoeckli Dec 16 '20 at 23:33
Thank you very much! I didn`t know c# had a library for BCrypt and I wanted to implement the system myself for educational purposes, but you are right, that is a much better option – Ciprian Florin Dec 17 '20 at 06:31

Why does my password checker not work? c#

2 Answers2