0

I am just wondering what all the security experts think of my method for password hashing. I want to come up with a method I can use for all my future web development projects.

First I create a random salt based on a 32 byte array :

RNGCryptoServiceProvider provider = new RNGCryptoServiceProvider();
byte[] randomNumber = new byte[32];
provider.GetBytes(randomNumber);
string salt = System.Text.Encoding.UTF8.GetString(randomNumber);

I would then store that salt in the user's database row.

To hash the password, I would use :

// Create a new instance of the RijndaelManaged
// class. This generates a new key and initialization
// vector (IV).
using (RijndaelManaged myRijndael = new RijndaelManaged())
{
myRijndael.GenerateKey();
myRijndael.GenerateIV();
// Encrypt the string to an array of bytes.
byte[] encrypted = EncryptStringToBytes(salt + password, myRijndael.Key, myRijndael.IV);
string encryptedPassword = System.Text.Encoding.UTF8.GetString(encrypted);
}

The EncryptStringToBytes and DecryptStringFromBytes methods can be seen here : http://msdn.microsoft.com/en-us/library/system.security.cryptography.rijndaelmanaged.aspx

If anyone could look this over and let me know of any vulnerabilities or places where I could strengthen it, I would greatly appreciate it! Thanks!

Edit

I've implemented a version of BCrypt here :

        string myPassword = "password";

        string mySalt = BCrypt.GenerateSalt();

        string myHash = BCrypt.HashPassword(myPassword, mySalt);
Chrisgozd
  • 83
  • 1
  • 6
  • 1
    1) You're using UTF-8 encoding on binary data. Thanks to unfortunate defaults in .net, that means they get corrupted silently. Use Base64 instead. 2) When you use UTF8Encoding, create your own instance, with throw-on-error enabled. That way you don't run into silent corruption. 3) I don't see any hash in your code 4) 32 bytes is overkill. Id use 16. – CodesInChaos Oct 14 '13 at 09:31
  • Besides the mistakes others have pointed out: There is never a **most** secure way to hash something. You can always add a longer salt or a hash algorithm with a longer key length to increase the exponent of the time it takes to brute-force. At one point you have reached a method which is **sufficiently** secure, but there is no theoretical upper limit. – Philipp Oct 14 '13 at 15:17

1 Answers1

11

No, no, no, no, no. Encryption is not hashing.

You need to use a strong hashing algorithm like bcrypt or pbkdf2 instead. You can use a library like Bcrypt.Net.

With the library, hashing a password is simply a matter of calling,

BCrypt.Net.BCrypt.HashPassword(password, workFactor);

Verification is just as easy,

BCrypt.Net.BCrypt.Verify(password, hashed_password);

If you do not want an additional dependency in your project, look into using the built in Rfc2898DeriveBytes class instead.

Community
  • 1
  • Ah, thank you. For the Rfc2898DeriveBytes, how many iterations would you recommend? - Also, do you think my salt creation method is substantial in creating a 32 byte salt? – Chrisgozd Oct 14 '13 at 02:59
  • @Chrisgozd Yes, `RNGCryptoServiceProvider` is a cryptographically secure RNG so you should be find there. With regards to iteration count, that depends on the hardware you are targeting. The correct answer will be as high as possible without introducing large delays for your users. So, profile profile profile. –  Oct 14 '13 at 03:02
  • Thanks, would you mind checking out my BCrypt edit up above? Thanks! – Chrisgozd Oct 14 '13 at 12:42
  • @Chrisgozd today you usually use 100'000 to 250'000 iterations for PBKDF2. – dom0 Oct 14 '13 at 17:29
  • @Chrisgozd What exactly is `BcryptDude`? I hope you are just wrapping the `Bcrypt.NET` library in your own namespace and not rolling your own version of the algorithm... –  Oct 15 '13 at 06:13
  • @TerryChia haha yes, that's what i'm doing, sorry I fixed it :) – Chrisgozd Oct 16 '13 at 12:03
  • @Chrisgozd It looks ok, but you don't actually have to generate the salt separately unless you want to store it elsewhere. The `HashPassword()` function will automatically generate the salt for you. –  Oct 16 '13 at 12:36