13

There are seveal ways (even here in SO) and they all mention that the best way to keep password on database is to save, not the password, not the hased password, but to store the hash of a salted password.

My question is simple, putting some code on it, is this the correct way?

string username = "myUsr";
string password = "myPwd";
DateTime createDate = DateTime.UtcNow;

// Salt it
string saltedPwd = String.Concat(password, createDate.Ticks.ToString());

// Hash it
HMACSHA1 hash = new HMACSHA1(Encoding.Unicode.GetBytes(Helper.EncryptKey));
string encodedPwd = Convert.ToBase64String(
                        hash.ComputeHash(Encoding.Unicode.GetBytes(saltedPwd)));

// Create User in the database
db.CreateUser(username, encodedPwd, createDate);

Database User Table

user_id | username | password | create_date | last_access | active

and upon Login use do the process again and check if the encodedPwd is the same as the salted, hased password that was provided.

My only concern is, is this the best way to salt a password? Is it ok to use the Created Date (as that will always change, and I read that it is best to use always a different salt every time we encode a password...

Or should be the salt a completely different variable?

balexandre
  • 73,608
  • 45
  • 233
  • 342
  • 2
    I'd say it's good, I'd be comfortable with it. It will be enough to defeat rainbow tables. – sipsorcery Mar 25 '11 at 10:46
  • How will you validate the password next time, your salt is **UtcNow** – V4Vendetta Mar 25 '11 at 10:52
  • @V4Vendetta That's the date that is appended to the `CreateDate` in the UserRow, so I can always check that latter... the date is placed in the Table as well. – balexandre Mar 25 '11 at 10:53
  • 1
    You might consider salting it with the user ID in addition. This prevents copying one password in the DB to another row, changing a user's password. – Krumelur Mar 25 '11 at 10:59
  • @balexandre: sorry missed that, my only concern is ToString() is culture specific as is **datetime** so need to be careful on that part – V4Vendetta Mar 25 '11 at 10:59
  • @V4Vendetta duly noted, maybe I can put the `.Ticks` in a new table column for this :) – balexandre Mar 25 '11 at 11:00

5 Answers5

20

Your implementation is probably good enough, but it would be better to use a salt with more entropy: the ticks value that you're currently using will always be in a relatively small range.

I would suggest using something like PBKDF2 to do the work for you, via Rfc2898DeriveBytes:

string username = "myUsr";
string password = "myPwd";

using (var deriveBytes = new Rfc2898DeriveBytes(password, 20)) // 20-byte salt
{
    byte[] salt = deriveBytes.Salt;
    byte[] key = deriveBytes.GetBytes(20); // 20-byte key

    string encodedSalt = Convert.ToBase64String(salt);
    string encodedKey = Convert.ToBase64String(key);

    // store encodedSalt and encodedKey in database
    // you could optionally skip the encoding and store the byte arrays directly
    db.CreateUser(username, encodedSalt, encodedKey);
}

And to authenticate...

string username = "myUsr";
string password = "myPwd";

string encodedSalt, encodedKey;
// load encodedSalt and encodedKey from database for the given username
byte[] salt = Convert.FromBase64String(encodedSalt);
byte[] key = Convert.FromBase64String(encodedKey);

using (var deriveBytes = new Rfc2898DeriveBytes(password, salt))
{
    byte[] testKey = deriveBytes.GetBytes(20); // 20-byte key

    if (!testKey.SequenceEqual(key))
        throw new InvalidOperationException("Password is invalid!");
}
LukeH
  • 263,068
  • 57
  • 365
  • 409
  • Wouldn't it be better to use a static salt key in the code, and not save to salt to the database? – Magnus Mar 25 '11 at 11:52
  • 2
    @Magnus: And allow someone who finds out the salt to create rainbow tables for *all* passwords used on that sit? ;) – ThiefMaster Mar 25 '11 at 12:17
  • Doesn't having the salt in the DB make it very easy to find? – Magnus Mar 25 '11 at 12:36
  • 5
    @Magnus: It doesn't matter that the salt is easy to find; what matters is that each user has a unique, random salt. A good authentication scheme should stand up even if the attacker has access to all the code and all the data. – LukeH Mar 25 '11 at 12:40
4

I'm wondering why nobody has mentioned BCrypt yet. There is a ready to use implementation for C#. See http://derekslager.com/blog/posts/2007/10/bcrypt-dotnet-strong-password-hashing-for-dotnet-and-mono.ashx

Don't reinvent the wheel if there are proofed solutions for your issue.

ccellar
  • 10,326
  • 2
  • 38
  • 56
  • Indeed. Key strengthening schemes like bcrypt or PBKDF2 are better than plain hashing. – LukeH Mar 25 '11 at 12:45
  • So... if it's an open source project, anyone can see the code, and the same way I can verify if it's a valid password, an hacker can do the same thing with he's rainbow list? – balexandre Mar 25 '11 at 13:27
  • 6
    @balexandre: I don't understand your concern here. **You should be using bcrypt or some other library written by professionals**. Never roll your own crypto code if you can possibly avoid it. It has been designed by experts and is very secure *even if the source code to the cryptosystem is known to attackers*. That's how secure it is. Also *do not use the time as a salt*. Use bytes derived from a cryptographically strong source of entropy for the salt, and store those bytes along with the hashed password. (Bcrypt does this automatically.) – Eric Lippert Mar 25 '11 at 13:56
  • @Eric Lippert As it's my first time I'm jumping this way of storing pwd, I think it's a legitimate concern :) – balexandre Mar 25 '11 at 15:06
0

Your method is totally fine, but let's say someone got your database, but not your code base. They could essentially figure out that you simply concatenated the password and create date, and they could reverse engineer all passwords.

You may want to further inject a unique string that only exists in your code base for a little extra protection.

string username = "myUsr";
string password = "myPwd";
DateTime createDate = DateTime.UtcNow;

// Salt it
string saltedPwd = String.Concat(password, SomeOtherClass.StaticKey, createDate.Ticks.ToString());

public class SomeOtherClass
{
    public static string StaticKey = "#$%#$%superuniqueblahal$#%@#$43580"; // should probably be const/readonly, but whatever
}
Langdon
  • 19,875
  • 18
  • 88
  • 107
  • Hmmm, debatable. At best it would let them provide the salt and then progress to a dictionary attack. How would they be able to see the creation date was used? Also, they wouldn't know how the creation date was formatted to create the salt. – Adam Houldsworth Mar 25 '11 at 10:50
  • They could Google the author's name and find this StackOverflow question. :) Or more realistically they could brute force a ton of common approaches using the data on the user row. – Langdon Mar 25 '11 at 10:53
  • Don't they need to know the `Helper.EncryptKey` (The Hash Key) as well so the Hash could be used? – balexandre Mar 25 '11 at 10:54
  • So there are two options, a mix of static and dynamic salts applied, or sprinkle the dynamic salt throughout the main password as part of the hash, instead of appending it to the front or back. – Adam Houldsworth Mar 25 '11 at 10:57
  • @Langdon But that `StaticKey` I use as the hash key, isn't that already good approach? do I need to append it for the salt as well? – balexandre Mar 25 '11 at 10:59
  • @balexandre The idea behind StaticKey is the same as Helper.EncryptKey, I glanced over that as I read. I'm used to using MD5 for hashing. – Langdon Mar 25 '11 at 11:03
0

How about you try: ProtectedData.Protect method?

This method can be used to encrypt data such as passwords, keys, or connection strings. The optionalEntropy parameter enables you to add data to increase the complexity of the encryption; specify null for no additional complexity. If provided, this information must also be used when decrypting the data using the Unprotect method.

KMån
  • 9,896
  • 2
  • 31
  • 41
  • Bad idea. You're still suggesting storing the password in the database, albeit encrypted. Steal the source code as well as the users table and you've got a means to decrypt everyone's passwords. – tomfanning Mar 25 '11 at 11:13
  • `ProtectedData` is used for encrypting and decrypting, not hashing. – LukeH Mar 25 '11 at 11:14
  • @tomfanning are you implying that a solution exists that will protect passwords in the event that "hackers" have both the database and source code? – Langdon Mar 25 '11 at 11:21
  • 1
    @Langdon: Yes. Hashing. If the password is never stored in the first place then "hackers" can't get at it. – LukeH Mar 25 '11 at 11:27
  • Yes, absolutely. More specifically, hashing with salt. Which is what this question is all about. Never store your passwords, full stop, (reversibly) encrypted or otherwise. – tomfanning Mar 25 '11 at 12:32
0

I think the idea with CreateDate is adequately powerful, but when someone steal your DB and Code, your salt is revealed. Security based on "nobody can snuff my code" is bad security.

You can simply double hash the password... And use salt from first hashing.

string Flavor(string passwd)
{
   string fhash = Str2SHA1(passwd);
   string salt = fhash[2] + fhash [10] + fhash[1]; // or whatever...
   string realhash = Str2SHA1(hash + salt);
}
string Str2Sha1(string str){ ... }
Jan 'splite' K.
  • 1,667
  • 2
  • 27
  • 34