Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Hash MD5, Sha1 and Length

700 views
Skip to first unread message

shapper

unread,
Sep 12, 2009, 10:08:18 PM9/12/09
to
Hello,

I created a String extension to hash using MD5 and SHA1:

public static String Hash(this String value, EncryptionType type)
{

// Hash value
Byte[] hash = null;
UTF8Encoding encoder = new UTF8Encoding();
switch (type) {
case EncryptionType.MD5:
MD5CryptoServiceProvider md5 = new MD5CryptoServiceProvider
();
hash = md5.ComputeHash(encoder.GetBytes(value));
break;
case EncryptionType.SHA1:
SHA1CryptoServiceProvider sha1 = new
SHA1CryptoServiceProvider();
hash = sha1.ComputeHash(encoder.GetBytes(value));
break;
}
return BitConverter.ToString(hash).Replace("-", String.Empty);

} // Hash

What is the relation between the original string length and the hashed
string lenth?

Thanks,
Miguel

Arne Vajhøj

unread,
Sep 12, 2009, 10:24:57 PM9/12/09
to

None.

A hash function converts all strings to a fixed length hash that only
depends on the algorithm

MD5 is 16 bytes. SHA1 is 20 bytes.

And you should not use any of those two !

Use SHA-256 (that will return 32 bytes).

Arne

Tom Spink

unread,
Sep 13, 2009, 2:34:18 AM9/13/09
to
Hello Miguel,

shapper wrote:

> Hello,
>
> I created a String extension to hash using MD5 and SHA1:

[snippedy]

In addition to Arne's excellent comment about *not* using MD5 and SHA1,
you should ideally refactor your code a bit to rename the
'EncryptionType' enumeration to 'HashType' - It's a bit misleading
because hashing is certainly not encrypting (as encryption implies the
ability to decrypt).

Also, the optimiser in me spots a way you could reduce code redundancy a
bit.

Since the hashing algorithms all derive from a base class,
'HashAlgorithm', you could do this:

///
HashAlgorithm algo;
switch (hashType) {
case HashType.MD5:
algo = new MD5CryptoServiceProvider();
break;
case HashType.SHA1:
algo = new SHA1CryptoServiceProvider();
break;
default:
throw new ArgumentException();
}

byte[] hash = algo.ComputeHash(encoder.GetBytes(value));
///

--
Tom

shapper

unread,
Sep 13, 2009, 11:42:14 AM9/13/09
to
So for saving user passwords on a database I should use Sha256
correct?
I was using MD5 or Sha1 because those were the options on the ASP.NET
Membership providers.
But now I creating my own custom Membership system based only on
FormsAuthentication.

I did a little search and I found a few more options and updated my
code with your suggestions:

public static String Hash(this String value, HashType hashType) {

HashAlgorithm algorithm;
switch (hashType) {
case HashType.MD5:
algorithm = new MD5CryptoServiceProvider();
break;
case HashType.SHA1:
algorithm = new SHA1CryptoServiceProvider();
break;
case HashType.SHA256:
algorithm = new SHA256CryptoServiceProvider();
break;
case HashType.SHA384:
algorithm = new SHA384CryptoServiceProvider();
break;
case HashType.SHA512:
algorithm = new SHA512CryptoServiceProvider();
break;
default:
throw new ArgumentException("Invalid hash type", "type");


}
UTF8Encoding encoder = new UTF8Encoding();

Byte[] hash = algorithm.ComputeHash(encoder.GetBytes(value));


return BitConverter.ToString(hash).Replace("-", String.Empty);

} // Hash

If you notice something wrong, please, let me know.

Thank You,
Miguel

Tom Spink

unread,
Sep 13, 2009, 11:57:29 AM9/13/09
to
Hi Miguel,

shapper wrote:
> So for saving user passwords on a database I should use Sha256
> correct?

Well... Arne's point was that MD5 and SHA1 are rubbish. And they are.
So you *should* use something stronger. SHA256 is a good choice.

> I did a little search and I found a few more options and updated my
> code with your suggestions:

[snip]

Looking good! Just a wee operational comment about extensibility:

I'd allow the caller to specify the encoding to use, then make an
overload that uses a default encoding:

/// (mind out for wrapped lines)


public static String Hash(this String value,

HashType hashType, Encoding encoding)
{
...
}

public static String Hash(this String value, HashType hashType)
{

return Hash(value, hashType, new UTF8Encoding());
}
///

That way, if in the future you want to use ASCII encoding, or whatever,
you don't have to break existing code.

> Thank You,
> Miguel
--
Tom

shapper

unread,
Sep 13, 2009, 12:30:18 PM9/13/09
to

That is a nice tip. So then I use, if I am not wrong:
Byte[] hash = algorithm.ComputeHash(encoding.GetBytes(value));

And for my password scenario what encoding would you use?
The password might have latin characters like ã, á, etc.

Thank You,
Miguel

rossum

unread,
Sep 13, 2009, 2:29:12 PM9/13/09
to
On Sun, 13 Sep 2009 08:42:14 -0700 (PDT), shapper <mdm...@gmail.com>
wrote:

>So for saving user passwords on a database I should use Sha256
>correct?

For saving passwords on a database you need to use SHA-256,
cryptographic salt (http://en.wikipedia.org/wiki/Salt_(cryptography))
and stretching (http://en.wikipedia.org/wiki/Key_strengthening).
These are standard cryptographic methods for protecting bulk passwords
in a database or similar. See PKCS#5
(ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-5v2/pkcs5v2-0.pdf), sections
4 and 5 for more detail.

If you are doing security then it is critical to get it right.

This is insufficient. Use something more like:

Byte[] hash = ComputeHash( password | salt ); // | = concatenate
for (int 1 = 0; i < 10000; ++i)
hash = ComputeHash( hash | salt ); // | = concatenate

The salt prevents two people using the same password having the same
hash and also stops an attacker pre-calculating hashes for commonly
used passwords. The repeats (10,000 or whatever) increase the
workload for any attacker trying to guess passwords. Aim to set the
number of repeats so that it takes about 0.25 seconds to calculate a
hash. That way the attacker can only try guessing four possible
passwords per second per PC, while users only get a minor delay when
signing on.

You will need to store the salts alongside the hashes as they are all
different. The repeat count is a single variable. It theory it
should be reviewed every few years and increased in line with
computing power. You should also review the hash function used to see
if it has become obsolete. NIST is currently running a competition to
find a new hash algorithm to replace the SHA2 series, which includes
SHA-256. Google "SHA 3 competition" if you are interested.

rossum

shapper

unread,
Sep 13, 2009, 2:56:40 PM9/13/09
to
On Sep 13, 7:29 pm, rossum <rossu...@coldmail.com> wrote:
> On Sun, 13 Sep 2009 08:42:14 -0700 (PDT), shapper <mdmo...@gmail.com>

Hi,

What you mean is the following correct:
http://www.obviex.com/samples/hash.aspx

Please, if you know better code to do this let me know.
I am not very familiar with this but I have been reading the links you
sent me and searched for the competition.

Then for each user I save in the database along with the resulting
password hash, its salt to I can use it later to compare the passwords
on authentication, correct?

Thanks,
Miguel

shapper

unread,
Sep 13, 2009, 4:34:30 PM9/13/09
to
Hello,

I tried to create the extension with and without salt and with
optional encoding.
I also made the salt as OUT so if the user does not provide a salt
value then it will be generated and updated.
This is useful to save the Salt in a database.

1. Can the salt parameter be a String?
Then inside the extension I would convert the salt to bytes[] or
create a random in bytes and at the end convert it to string.
I think it would be more user friendly.

2. What is the difference between using SHA256Managed() or
SHA256CryptoServiceProvider() ?
Which one should I use?

Could someone, please, help me improve my code?
I am not so familiar with Cryptography ...

This is the code I have so far:

public static String Hash(this String value, HashType type) {
return Hash(value, type, new UTF8Encoding());
} // Hash

public static String Hash(this String value, HashType type,
Encoding encoding) {

// Hash value
HashAlgorithm algorithm;
switch (type) {


case HashType.MD5:
algorithm = new MD5CryptoServiceProvider();
break;
case HashType.SHA1:
algorithm = new SHA1CryptoServiceProvider();
break;
case HashType.SHA256:
algorithm = new SHA256CryptoServiceProvider();
break;
case HashType.SHA384:
algorithm = new SHA384CryptoServiceProvider();
break;
case HashType.SHA512:
algorithm = new SHA512CryptoServiceProvider();
break;
default:
throw new ArgumentException("Invalid hash type", "type");
}

Byte[] hash = algorithm.ComputeHash(encoding.GetBytes(value));


return BitConverter.ToString(hash).Replace("-", String.Empty);

} // Hash

public static String Hash(this String value, HashType type, out
Byte[] salt) {
return Hash(value, type, out salt, new UTF8Encoding());
} // Hash

public static String Hash(this String value, HashType type, out
Byte[] salt, Encoding encoding) {

// Check salt
if (salt == null) {

// Define a random salt
Random random = new Random();
Int32 size = random.Next(4, 8);
salt = new Byte[size];
RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider();
rng.GetNonZeroBytes(salt);

}

// Define salted
Byte[] valueBytes = encoding.GetBytes(value);
Byte[] valueSaltedBytes = new Byte[valueBytes.Length +
valueBytes.Length];
for (Int32 i = 0; i < valueBytes.Length; i++)
valueSaltedBytes[i] = valueBytes[i];
for (Int32 i = 0; i < salt.Length; i++)
valueSaltedBytes[valueBytes.Length + i] = salt[i];

// Hash value
HashAlgorithm algorithm;
switch (type) {


case HashType.MD5:
algorithm = new MD5CryptoServiceProvider();
break;
case HashType.SHA1:
algorithm = new SHA1CryptoServiceProvider();
break;
case HashType.SHA256:
algorithm = new SHA256CryptoServiceProvider();
break;
case HashType.SHA384:
algorithm = new SHA384CryptoServiceProvider();
break;
case HashType.SHA512:
algorithm = new SHA512CryptoServiceProvider();
break;
default:
throw new ArgumentException("Invalid hash type", "type");
}

// Define hash
Byte[] hash = algorithm.ComputeHash(valueSaltedBytes);
Byte[] hashSalted = new Byte[hash.Length + salt.Length];
for (Int32 i = 0; i < hash.Length; i++)
hashSalted[i] = hash[i];
for (Int32 i = 0; i < salt.Length; i++)
hashSalted[hash.Length + i] = salt[i];

// Return hash
return Convert.ToBase64String(hashSalted);

} // Hash

Thank You!
Miguel

Arne Vajhøj

unread,
Sep 13, 2009, 4:49:50 PM9/13/09
to
shapper wrote:
> So for saving user passwords on a database I should use Sha256
> correct?

I think so.

Several serious weaknesses have been found in MD5, some has been found
in SHA1 and more are expected for SHA1.

Most of these vulnerabilities does not apply to your context of
hashed passwords.

But given that SHA256 is just as easy to use as MD5 and SHA1, then I
would say that: unless you are an expert in cryptography and has read
and understood all the vulnerabilities and are sure they do not apply,
then you should go for a hash algorithm that is considered safe in
general.

> UTF8Encoding encoder = new UTF8Encoding();
> Byte[] hash = algorithm.ComputeHash(encoder.GetBytes(value));
> return BitConverter.ToString(hash).Replace("-", String.Empty);

You could use Encoding.UTF8 instead of new UTF8Encoding.

And you could store the hash as a binary instead of as a string.

Or you could do as you do.

Arne

Arne Vajhøj

unread,
Sep 13, 2009, 4:57:46 PM9/13/09
to
shapper wrote:
> I tried to create the extension with and without salt and with
> optional encoding.
> I also made the salt as OUT so if the user does not provide a salt
> value then it will be generated and updated.
> This is useful to save the Salt in a database.

The salt should not be provided by the user. It should be
unpredictable random.

> 1. Can the salt parameter be a String?

It can.

Just be sure that it can and will take a lot of different values.

> Then inside the extension I would convert the salt to bytes[] or
> create a random in bytes and at the end convert it to string.
> I think it would be more user friendly.

The user should never see that salt, so it does not matter.

> 2. What is the difference between using SHA256Managed() or
> SHA256CryptoServiceProvider() ?

The first is pure .NET while the second calls some Win32 native code.

> Which one should I use?

I would use the pure .NET, but I don't think it matters much.

Arne

Jesse Houwing

unread,
Sep 13, 2009, 6:03:30 PM9/13/09
to

Don't forget that *CryptoServiceProvider classes all implement
IDisposable and should be properly disposed after use!

so

HashAlgorithm algorithm = null;
try
{
... your code
} finally
{
if (algorithm != mull)
{
algorithm.Dispose();
}
}

--
Jesse Houwing
jesse.houwing at sogeti.nl

shapper

unread,
Sep 13, 2009, 6:09:23 PM9/13/09
to
On Sep 13, 9:57 pm, Arne Vajhøj <a...@vajhoej.dk> wrote:
> The salt should not be provided by the user. It should be
> unpredictable random.

Yes, I know. When I said provided I meant provided from outside the
extension and not generated inside the extension.
I could also create a new extension/method to create random salts
given a minimum and maximum length.

> The user should never see that salt, so it does not matter.

Yes, my intention is more to save it as byte or string in the
database.
I was thinking in string because in ASP.NET Membership providers they
use nvarchar to save the salt.

> I would use the pure .NET, but I don't think it matters much.

Ok, I will use the pure.

Thanks,
Miguel

rossum

unread,
Sep 13, 2009, 6:10:07 PM9/13/09
to
On Sun, 13 Sep 2009 11:56:40 -0700 (PDT), shapper <mdm...@gmail.com>
wrote:

>Hi,


>
>What you mean is the following correct:
>http://www.obviex.com/samples/hash.aspx

This is not good cryptographically, though it is a reasonable teaching
example. It gives too many options for hash functions, it contains no
stretching/strengthening and it is weak on salts by allowing short
salts and less than random contents.

>
>Please, if you know better code to do this let me know.
>I am not very familiar with this but I have been reading the links you
>sent me and searched for the competition.

In pseudocode:

generateHashAndSalt( user, password )
salt <- generateRandomSalt()
save salt to database indexed by user
finalHash <- calculateHash( password, salt )
overwrite password array with zeros
save finalHash to database indexed by user
end generateHash

REPEATS <- 10,000

byte[] calculateHash( password, salt )
hash <- SHA256( concatenate( password, salt ) )
for REPEATS times do
hash <- SHA256( concatenate( hash, salt ) )
end for
return hash
end calculateHash

All salts should be the same length, 128 bits (16 bytes) is fine. If
space is very tight in your database then you could drop as far as 64
bits (8 bytes) but that is much less secure. The cost of losing your
data is probably a lot more than the cost of some extra disk space.
Use a cryptographic RNG; RNGCryptoServiceProvider.GetBytes() is the
obvious method. Do not use the ordinary Random as that is not
cryptographically secure.

The value of REPEATS needs to be set so that the hash calculation
takes about 0.25 second on whatever machine you are using.

Overwriting the password array is a standard security precaution. It
is the rough equivalent of displaying the password as asterisks on
screen.

To check a hash:

boolean checkHash( user, password )
salt <- retrieveSalt( user )
newHash <- calculateHash( password, salt )
overwrite password array with zeros
oldHash <- retrieveHash( user )
return ( oldHash = newHash )
end checkHash

>
>Then for each user I save in the database along with the resulting
>password hash, its salt to I can use it later to compare the passwords
>on authentication, correct?

Correct.

rossum

>
>Thanks,
>Miguel

shapper

unread,
Sep 13, 2009, 6:30:59 PM9/13/09
to
On Sep 13, 11:10 pm, rossum <rossu...@coldmail.com> wrote:

> This is not good cryptographically, though it is a reasonable teaching
> example.  It gives too many options for hash functions, it contains no
> stretching/strengthening and it is weak on salts by allowing short
> salts and less than random contents.

It is difficult for me to identify what a good example is to follow.
Is this closer of what you mean:
http://www.dijksterhuis.org/creating-salted-hash-values-in-c/

And what do you mean with stretching/strengthening?

> In pseudocode:
>
>   generateHashAndSalt( user, password )
>     salt <- generateRandomSalt()
>     save salt to database indexed by user
>     finalHash <- calculateHash( password, salt )
>     overwrite password array with zeros
>     save finalHash to database indexed by user
>   end generateHash

Got it ... At the moment my main problem is creating the Hash / Salt
Algorithm.
What do you mean overwrite password array with zeros? I got lost.

>   REPEATS <- 10,000
>
>   byte[] calculateHash( password, salt )
>     hash <- SHA256( concatenate( password, salt ) )
>     for REPEATS times do
>        hash <- SHA256( concatenate( hash, salt ) )
>     end for
>     return hash
>   end calculateHash  

Got it.

> All salts should be the same length, 128 bits (16 bytes) is fine.  If
> space is very tight in your database then you could drop as far as 64
> bits (8 bytes) but that is much less secure.  

I don't have a problem with space. Will follow your advice.

> Use a cryptographic RNG; RNGCryptoServiceProvider.GetBytes() is the
> obvious method.  Do not use the ordinary Random as that is not
> cryptographically secure.

I was am using it on my code taken from the first url I posted:

RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider();
rng.GetNonZeroBytes(salt);

> The value of REPEATS needs to be set so that the hash calculation


> takes about 0.25 second on whatever machine you are using.

Ok. First let me try to create the Hash code.

> Overwriting the password array is a standard security precaution.  It
> is the rough equivalent of displaying the password as asterisks on
> screen.

I don't understand ...

> To check a hash:
>
>   boolean checkHash( user, password )
>     salt <- retrieveSalt( user )
>     newHash <- calculateHash( password, salt )
>     overwrite password array with zeros
>         oldHash <- retrieveHash( user )
>     return ( oldHash = newHash )
>   end checkHash

Will try when I have the code made.

Thanks,
Miguel

rossum

unread,
Sep 13, 2009, 6:31:51 PM9/13/09
to
On Sun, 13 Sep 2009 13:34:30 -0700 (PDT), shapper <mdm...@gmail.com>
wrote:

>Hello,


>
>I tried to create the extension with and without salt and with
>optional encoding.

Salt must always be present, either generated or retrieved form the
database. Reduce the number of options available, that way the users
have less chances of picking the wrong options.

>I also made the salt as OUT so if the user does not provide a salt
>value then it will be generated and updated.

Always generate the salt internally, the users never need to see it.
You generate it, you save it to the database and you retrieve it when
needed.

>This is useful to save the Salt in a database.
>
>1. Can the salt parameter be a String?

Unwise. A string encoding is not completely random and a salt should
be as random as possible.


> Then inside the extension I would convert the salt to bytes[] or
>create a random in bytes and at the end convert it to string.
> I think it would be more user friendly.
>
>2. What is the difference between using SHA256Managed() or
>SHA256CryptoServiceProvider() ?

Pass.

You need to stretch the calculation by repeating it many times.


> return BitConverter.ToString(hash).Replace("-", String.Empty);
>
> } // Hash

Do not allow the user to select the hash function. They can have any
hash function they want as long as it is SHA256.


>
> public static String Hash(this String value, HashType type, out
>Byte[] salt) {
> return Hash(value, type, out salt, new UTF8Encoding());
> } // Hash
>
> public static String Hash(this String value, HashType type, out
>Byte[] salt, Encoding encoding) {
>
> // Check salt
> if (salt == null) {
>
> // Define a random salt
> Random random = new Random();
> Int32 size = random.Next(4, 8);
> salt = new Byte[size];
> RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider();
> rng.GetNonZeroBytes(salt);

This is needless complication again. Short salts are dangerous. Pick
a reasonable salt size and stick with it:

final static int SALT_SIZE = 16;
byte[] salt = new byte[SALT_SIZE];


RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider();

rng.GetBytes(salt);

It is a minor error to use the GetNonZeroBytes() method in this
context as it reduces the randomness of the salt.

Again, give the user no choice. SHA256 only.


>
> // Define hash
> Byte[] hash = algorithm.ComputeHash(valueSaltedBytes);
> Byte[] hashSalted = new Byte[hash.Length + salt.Length];
> for (Int32 i = 0; i < hash.Length; i++)
> hashSalted[i] = hash[i];
> for (Int32 i = 0; i < salt.Length; i++)
> hashSalted[hash.Length + i] = salt[i];
>
> // Return hash
> return Convert.ToBase64String(hashSalted);

Base64 increases both processing time and the storage space needed.
Unless you need to transmit the data over a text-only link then stick
with byte arrays.

rossum

shapper

unread,
Sep 13, 2009, 7:15:25 PM9/13/09
to
On Sep 13, 11:31 pm, rossum <rossu...@coldmail.com> wrote:

> Salt must always be present, either generated or retrieved form the
> database.  Reduce the number of options available, that way the users
> have less chances of picking the wrong options.

I am creating a simple console application and following what you told
me but until now I don't get two equal hashes:

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

public class Program {

public static void Main(String[] args) {

String password = "password";
Byte[] hash;
Byte[] salt;
Hash(password, out hash, out salt);

String resultHash = Convert.ToBase64String(hash);
String resultSalt = Convert.ToBase64String(salt);

Console.WriteLine("Hash:");
Console.WriteLine(resultHash);
Console.WriteLine("Salt:");
Console.WriteLine(resultSalt);

Byte[] hash2;
Hash(password, out hash2, out salt);

Console.WriteLine("Compares hashes:");
Console.WriteLine((hash == hash2).ToString());
Console.WriteLine("Press any key...");
Console.ReadLine();

} // Main

public static void Hash(String password, out Byte[] hash, out Byte
[] salt) {

Int32 saltLength = 32;

Byte[] passwordBytes = Encoding.UTF8.GetBytes(password);
salt = new Byte[saltLength];

RNGCryptoServiceProvider random = new RNGCryptoServiceProvider
();
random.GetBytes(salt);

Byte[] passwordAndSalt = new Byte[password.Length + saltLength];

Array.Copy(passwordBytes, passwordAndSalt,
passwordBytes.Length);
Array.Copy(salt, 0, passwordAndSalt, passwordBytes.Length,
saltLength);

for (Int32 i = 1; i < 10000; i++) {

}

HashAlgorithm algorithm = new SHA256CryptoServiceProvider();
hash = algorithm.ComputeHash(passwordAndSalt);

} // Hash

} // Program

I am not really sure what I am doing wrong and I am not sure about the
loop you mentioned.

shapper

unread,
Sep 13, 2009, 7:42:00 PM9/13/09
to
Hi,

I added a StopWatch to check the time that it takes in calculation to
check as you mention for 0.25s and tried to implement the loop and
correct the errors but I keep having different hashes at the end.

At the moment this is what I have:


// Namespaces
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text;

using System.Security.Cryptography;

public class Program {

public static void Main(String[] args) {

String password = "password";

Byte[] hash = new Byte[32];
Byte[] salt = new Byte[64];

Stopwatch watch = new Stopwatch();


for (Int32 i = 1; i < 10000; i++) {

if (i == 200) watch.Start();


Hash(password, out hash, out salt);
}

watch.Stop();
TimeSpan ts = watch.Elapsed;
String time = String.Format("{0:00}:{1:00}:{2:00}.{3:00}",
ts.Hours, ts.Minutes, ts.Seconds, ts.Milliseconds / 10);

Console.WriteLine("");
Console.WriteLine(time);
Console.WriteLine((ts.TotalMilliseconds / (20000 - 200)).ToString
());
Console.WriteLine("");

String resultHash = Convert.ToBase64String(hash);
String resultSalt = Convert.ToBase64String(salt);

Console.WriteLine("");
Console.WriteLine("Hash:");
Console.WriteLine(resultHash);
Console.WriteLine("");
Console.WriteLine("Salt:");
Console.WriteLine(resultSalt);
Console.WriteLine("");

Byte[] hash2;
Hash(password, out hash2, out salt);

Console.WriteLine("");


Console.WriteLine("Compares hashes:");
Console.WriteLine((hash == hash2).ToString());
Console.WriteLine("Press any key...");
Console.ReadLine();

} // Main

public static void Hash(String password, out Byte[] hash, out Byte
[] salt) {

Int32 saltLength = 32;

Byte[] passwordBytes = Encoding.UTF8.GetBytes(password);
salt = new Byte[saltLength];

RNGCryptoServiceProvider random = new RNGCryptoServiceProvider
();
random.GetBytes(salt);

Byte[] passwordAndSalt = new Byte[password.Length + saltLength];

Array.Copy(passwordBytes, passwordAndSalt,
passwordBytes.Length);
Array.Copy(salt, 0, passwordAndSalt, passwordBytes.Length,
saltLength);

HashAlgorithm algorithm = new SHA256CryptoServiceProvider();
hash = algorithm.ComputeHash(passwordAndSalt);

for (Int32 i = 1; i < 10000; i++) {

Array.Copy(salt, hash, salt.Length);
hash = algorithm.ComputeHash(hash);

}
} // Hash
} // Program

Does anyone knows what am I doing wrong?

Thanks,
Miguel

Peter Duniho

unread,
Sep 13, 2009, 8:39:38 PM9/13/09
to
On Sun, 13 Sep 2009 16:15:25 -0700, shapper <mdm...@gmail.com> wrote:

> On Sep 13, 11:31 pm, rossum <rossu...@coldmail.com> wrote:
>
>> Salt must always be present, either generated or retrieved form the
>> database.  Reduce the number of options available, that way the users
>> have less chances of picking the wrong options.
>
> I am creating a simple console application and following what you told
> me but until now I don't get two equal hashes:

Why should you get two equal hashes? The input to the hash function isn't
the same (different "salt" each time), so the likelihood of the output
being the same is incredibly low.

> [...]


> I am not really sure what I am doing wrong and I am not sure about the
> loop you mentioned.

He's saying that once you've hashed the original password, to repeat the
algorithm on the hash itself as many times as it takes to spend roughly
1/4 second on the repetitions.

Pete

Peter Duniho

unread,
Sep 13, 2009, 8:44:51 PM9/13/09
to
On Sun, 13 Sep 2009 15:03:30 -0700, Jesse Houwing
<jesse....@newsgroup.nospam> wrote:

> [...]


> HashAlgorithm algorithm = null;
> try
> {
> ... your code
> } finally
> {
> if (algorithm != mull)
> {
> algorithm.Dispose();
> }
> }

For what it's worth, this is a perfect example of when the "using"
statement is useful. :)

For example:

HashAlgorithm algorithm;

// initialize "algorithm" as needed

using (algorithm)
{
// use "algorithm"
}

Even better is if the selection is factored out into a separate method,
then the initialization of the "algorithm" variable can take place in the
"using" statement itself. (This latter point may be moot once Miguel has
changed his implementation to always use SHA256 :) ).

Pete

rossum

unread,
Sep 14, 2009, 5:12:10 AM9/14/09
to
On Sun, 13 Sep 2009 15:30:59 -0700 (PDT), shapper <mdm...@gmail.com>
wrote:

>On Sep 13, 11:10 pm, rossum <rossu...@coldmail.com> wrote:


>
>> This is not good cryptographically, though it is a reasonable teaching
>> example.  It gives too many options for hash functions, it contains no
>> stretching/strengthening and it is weak on salts by allowing short
>> salts and less than random contents.
>
>It is difficult for me to identify what a good example is to follow.
>Is this closer of what you mean:
>http://www.dijksterhuis.org/creating-salted-hash-values-in-c/

Three points:
1 It allows the user to select algorithms.
2 It does not use stretching/strengthening.
3 The Base-64 conversion is not needed.

>
>And what do you mean with stretching/strengthening?

That is the repetition of the hash calculation for many times to make
it take 0.25 second. Basically the for-loop.

[snip rest]

rossum

rossum

unread,
Sep 14, 2009, 7:01:25 AM9/14/09
to
On Sun, 13 Sep 2009 15:30:59 -0700 (PDT), shapper <mdm...@gmail.com>
wrote:

>What do you mean overwrite password array with zeros? I got lost.
Whoops, I missed that bit.

Very simple:

for (int i = 0; i < password.Length; ++i) {
password[i] = 0;
}

Jesse and Peter's suggestion about using using {} or Dispose() is also
excellent. The general idea is to leave as little as possible
sensitive information in your computer's memory when your program
terminates.

rossum

shapper

unread,
Sep 14, 2009, 10:50:32 AM9/14/09
to
On Sep 14, 12:01 pm, rossum <rossu...@coldmail.com> wrote:
> On Sun, 13 Sep 2009 15:30:59 -0700 (PDT), shapper <mdmo...@gmail.com>

> wrote:
>
> >What do you mean overwrite password array with zeros? I got lost.
>
> Whoops, I missed that bit.
>
> Very simple:
>
>   for (int i = 0; i < password.Length; ++i) {
>     password[i] = 0;
>   }
>
> Jesse and Peter's suggestion about using using {} or Dispose() is also
> excellent.  The general idea is to leave as little as possible
> sensitive information in your computer's memory when your program
> terminates.
>
> rossum

I am completely lost ... I have been following your tips but I am not
sure if what I have done is ok ... I suspect it isn't.
I am not able to use the watch to check the performance.
I tried to implement the stretching/strengthening by recalling the
ComputHash method.
I implemented IDisposable ... well I tried. :-)

I allow to pass the algorithm. Yes, I will use Sha256 but I would like
to leave that option. The same with length.
But I am testing and will use what you suggested me: Sha256 and Salt
with 16 bits

Could, someone, please help me? I am really lost on this.

// Namespaces
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text;
using System.Security.Cryptography;

// Test
namespace Test {

// Program
public class Program {

// Main


public static void Main(String[] args) {

String password = "hello";
Byte[] data = Encoding.UTF8.GetBytes(password);
Byte[] hash;
Byte[] salt;
SaltedHash test = new SaltedHash(new SHA256CryptoServiceProvider
(), 16);

//Stopwatch watch = new Stopwatch();
//for (Int32 i = 1; i < 1; i++) {
// if (i == 200) watch.Start();
// test.GetHashAndSalt(data, out hash, out salt);
//}
//watch.Stop();
//TimeSpan ts = watch.Elapsed;
//String time = String.Format("{0:00}:{1:00}:{2:00}.{3:00}",


ts.Hours, ts.Minutes, ts.Seconds, ts.Milliseconds / 10);

//Console.WriteLine("");
//Console.WriteLine(time);
//Console.WriteLine((ts.TotalMilliseconds / (20000 -
200)).ToString());
//Console.WriteLine("");

Console.WriteLine("");
test.GetHashAndSalt(data, out hash, out salt);
Console.WriteLine("Password = {0} , Hash = {1} , Salt = {2}",
Convert.ToBase64String(data), Convert.ToBase64String(hash),
Convert.ToBase64String(salt));
Console.WriteLine("");
Console.WriteLine("");
SaltedHash confirm = new SaltedHash(new
SHA256CryptoServiceProvider(), 32);
confirm.GetHashAndSalt(data, out hash, out salt);
Console.WriteLine("Password = {0} , Hash = {1} , Salt = {2}",
Convert.ToBase64String(data), Convert.ToBase64String(hash),
Convert.ToBase64String(salt));
Console.WriteLine("");
Console.WriteLine("");


Console.WriteLine("Press any key...");
Console.ReadLine();

} // Main


public class SaltedHash : IDisposable {

private HashAlgorithm HashProvider;
private Int32 SalthLength;

public SaltedHash(HashAlgorithm hashAlgorithm, Int32 saltLength)
{
HashProvider = hashAlgorithm;
SalthLength = saltLength;
}

public SaltedHash() : this(new SHA256Managed(), 16) { }

// GetHashAndSalt
public void GetHashAndSalt(Byte[] Data, out Byte[] Hash, out Byte
[] Salt) {

using (HashProvider) {

Salt = new byte[SalthLength];
RNGCryptoServiceProvider random = new
RNGCryptoServiceProvider();
random.GetNonZeroBytes(Salt);
Hash = ComputeHash(Data, Salt);

for (Int32 i = 1; i < 1000; i++) {
Hash = ComputeHash(Hash, Salt);
}

for (Int32 j = 0; j < Data.Length; ++j) {
Data[j] = 0;
}

}
} // GetHashAndSalt

// ComputeHash
private Byte[] ComputeHash(Byte[] Data, Byte[] Salt) {

Byte[] DataAndSalt = new Byte[Data.Length + SalthLength];

Array.Copy(Data, DataAndSalt, Data.Length);
Array.Copy(Salt, 0, DataAndSalt, Data.Length, SalthLength);

return HashProvider.ComputeHash(DataAndSalt);

} // ComputeHash

// Dispose
public void Dispose() {
this.Dispose();
GC.SuppressFinalize(this);
} // Dispose

}

} // Program
} // Test

shapper

unread,
Sep 14, 2009, 10:56:05 AM9/14/09
to
A side note adding to the code I just posted:

I will save the password and salt as binary on Users SQL table as
suggested before. I am planning to use the following:
Password varbinary(128) not null,
Salt varbinary(128) not null

Do you think it is ok?

Thanks,
Miguel


rossum

unread,
Sep 14, 2009, 2:12:09 PM9/14/09
to
On Mon, 14 Sep 2009 07:50:32 -0700 (PDT), shapper <mdm...@gmail.com>
wrote:

>On Sep 14, 12:01 pm, rossum <rossu...@coldmail.com> wrote:


>> On Sun, 13 Sep 2009 15:30:59 -0700 (PDT), shapper <mdmo...@gmail.com>
>> wrote:
>>
>> >What do you mean overwrite password array with zeros? I got lost.
>>
>> Whoops, I missed that bit.
>>
>> Very simple:
>>
>>   for (int i = 0; i < password.Length; ++i) {
>>     password[i] = 0;
>>   }
>>
>> Jesse and Peter's suggestion about using using {} or Dispose() is also
>> excellent.  The general idea is to leave as little as possible
>> sensitive information in your computer's memory when your program
>> terminates.
>>
>> rossum
>
>I am completely lost ... I have been following your tips but I am not
>sure if what I have done is ok ... I suspect it isn't.
>I am not able to use the watch to check the performance.
>I tried to implement the stretching/strengthening by recalling the
>ComputHash method.
>I implemented IDisposable ... well I tried. :-)
>
>I allow to pass the algorithm. Yes, I will use Sha256 but I would like
>to leave that option. The same with length.
>But I am testing and will use what you suggested me: Sha256 and Salt
>with 16 bits
>
>Could, someone, please help me? I am really lost on this.

[snip code]

It is difficult to jump into the middle of someone else's code. Try
to write a simplified cut-down version of your own. Once the
simplified version is working correctly then add the complications to
get it up to production standard.

Here is my very simple implementation. It does not use IDisposable,
though it does use "using () {}" once, and it does not clear
passwords. Nor does it use a database; salts and expected values are
passed in as parameters. Parameter validation and documentation
comments are also omitted. This code is not production quality.


public sealed class HashStuff {

private const int REPEATS = 25000;

private const int SALT_SIZE = 16;

private static RNGCryptoServiceProvider rng =
new RNGCryptoServiceProvider();

private static SHA256Managed sha;


private HashStuff() { } // Prevent instantiation


public static byte[] CalculateHash(byte[] password, byte[] salt) {
// Check parameters for nulls and salt size here.
byte[] hash;
using (sha = new SHA256Managed()) {
hash = sha.ComputeHash(Concat(password, salt));
for (int i = 0; i < REPEATS; ++i) {
hash = sha.ComputeHash(Concat(hash, salt));
}
} // end using
return hash;
}

public static byte[] GenerateSalt() {


byte[] salt = new byte[SALT_SIZE];

rng.GetBytes(salt);
return salt;
}

public static Boolean CheckHash(byte[] password,
byte[] salt, byte[] expected) {
// Check parameters for nulls, salt size and expected length.
byte[] actual = CalculateHash(password, salt);
for (int i = 0; i < actual.Length; ++i) {
if (actual[i] != expected[i]) { return false; }
}
return true;
}

private static byte[] Concat(byte[] lhs, byte[] rhs) {
byte[] result = new byte[lhs.Length + rhs.Length];
Array.Copy(lhs, result, lhs.Length);
Array.Copy(rhs, 0, result, lhs.Length, rhs.Length);
return result;
}

} // end class HashStuff


I ran some very simple tests on it using this code:


static class HashTest {

public static void Main() {

// 1 Test matching
byte[] password1 = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7 };
byte[] salt = HashStuff.GenerateSalt();
byte[] firstHash = HashStuff.CalculateHash(password1, salt);
byte[] password2 = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7 };
if (HashStuff.CheckHash(password2, salt, firstHash)) {
Console.WriteLine("Matching test OK");
} else {
Console.WriteLine("Matching test failed.");
}

// 2 Test not matching
password1 = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7 };
salt = HashStuff.GenerateSalt();
firstHash = HashStuff.CalculateHash(password1, salt);
password2 = new byte[] { 1, 1, 2, 3, 4, 5, 6, 7 };
if (HashStuff.CheckHash(password2, salt, firstHash)) {
Console.WriteLine("Not matching test failed.");
} else {
Console.WriteLine("Not matching test OK.");
}

// 3 Timer test
Stopwatch sw = new Stopwatch();
int numTests = 200;
sw.Start();
for (int i = 0; i < numTests; ++i) {
password1[0] = (byte)i;
firstHash = HashStuff.CalculateHash(password1, salt);
} // end for
sw.Stop();
Console.WriteLine("{0} tests took {1} mSec.", numTests,
sw.ElapsedMilliseconds);
Console.WriteLine("Average {0} seconds per hash.",
sw.ElapsedMilliseconds / (numTests * 1000.0));

// Keep console on screen
Console.Write("Press [Enter] to continue... ");
Console.ReadLine();
} // end Main()

} // end class HashTest


With these settings (REPEATS = 25000) on my machine I got:

Matching test OK
Not matching test OK.
200 tests took 44988 mSec.
Average 0.22494 seconds per hash.
Press [Enter] to continue...

HTH

rossum

Arne Vajhøj

unread,
Sep 14, 2009, 10:23:24 PM9/14/09
to
rossum wrote:
> The value of REPEATS needs to be set so that the hash calculation
> takes about 0.25 second on whatever machine you are using.

That comes as a cost.

4 logins (attempts) per second cost a CPU core.

For high volume it costs. And besides the increase in DoS
vulnerability may be more important than the added protection
against exploiting a stolen database.

Arne


Arne Vajhøj

unread,
Sep 14, 2009, 10:27:35 PM9/14/09
to
rossum wrote:
> It is a minor error to use the GetNonZeroBytes() method in this
> context as it reduces the randomness of the salt.

It removes one possible value out of 2^128.

It does not matter.

> Base64 increases both processing time and the storage space needed.

Given that you want to hash recursively for 0.25 seconds, then
the processing time spend on Base64 encoding will be insignificant.

Arne

rossum

unread,
Sep 15, 2009, 6:56:53 AM9/15/09
to
On Mon, 14 Sep 2009 22:23:24 -0400, Arne Vajhøj <ar...@vajhoej.dk>
wrote:

>rossum wrote:
>> The value of REPEATS needs to be set so that the hash calculation
>> takes about 0.25 second on whatever machine you are using.
>
>That comes as a cost.
>
>4 logins (attempts) per second cost a CPU core.

Correct, security costs. If you want the security then you pay the
cost. To quote Bruce Schneier: "There are already enough insecure
fast systems, we don't need another one." How much will it cost if an
attacker gets access to the passwords? How much will it cost to add
some more CPU to the server farm? Those are questions for the OP to
answer. If a court case ensues after loss of data then it is
important to be able to show that best security practice was being
used.

>
>For high volume it costs. And besides the increase in DoS
>vulnerability may be more important than the added protection
>against exploiting a stolen database.

That is the sort of thing that the OP has to assess. We do not know
that value of the data protected by the passwords.

rossum

>
>Arne
>

shapper

unread,
Sep 15, 2009, 8:18:37 AM9/15/09
to
On Sep 15, 11:56 am, rossum <rossu...@coldmail.com> wrote:
> On Mon, 14 Sep 2009 22:23:24 -0400, Arne Vajhøj <a...@vajhoej.dk>

In this case the most important data I have for each user its their
email ... In this case security is not a "big issue" ...
But I would like to have a secure and correct way to do this ...

I can then, from project to project, change the REPEATS from 25 000 to
10 000 or just 1 according to the level of security and processor cost
for each situation ...

Rossum,

Thank you for the code. I am just going to add a few things test it
and then post what I did here.

shapper

unread,
Sep 15, 2009, 1:11:04 PM9/15/09
to
Hello,

I added a few things to your code:

I defined HashAlgorithm, Repeats and SaltSize as inputs.

I didn't define them as properties since on your code you didn't use
any property and have the class sealed.
I am not sure if there is a security reason for this or something
else ...

I defined these parameters so I can use different configurations in
different projects if I need it.

I also cleanup the data input (password) on CalculateHash before I
return the hash.

And I tried to implement IDisposable but I am not exactly sure how to
do it and how to use it correctly.
I know it has to do with disposing managed and unmanaged resources but
I am not sure how to integrate it in this class.


shapper

unread,
Sep 15, 2009, 1:12:04 PM9/15/09
to

And I forgot to post the code I have know (I tested it and got the
same results when I ran you code):


using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text;
using System.Security.Cryptography;

namespace HashTestNamespace {

static class HashTest {

public static void Main() {

// 1 Test matching
byte[] password1 = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7 };

byte[] salt = HashStuff.GenerateSalt(16);
byte[] firstHash = HashStuff.CalculateHash(new SHA256Managed(),
25000, password1, salt);


byte[] password2 = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7 };

if (HashStuff.CheckHash(new SHA256Managed(), 25000, password2,


salt, firstHash)) {
Console.WriteLine("Matching test OK");
} else {
Console.WriteLine("Matching test failed.");
}

// 2 Test not matching
password1 = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7 };

salt = HashStuff.GenerateSalt(16);
firstHash = HashStuff.CalculateHash(new SHA256Managed(), 25000,


password1, salt);
password2 = new byte[] { 1, 1, 2, 3, 4, 5, 6, 7 };

if (HashStuff.CheckHash(new SHA256Managed(), 25000, password2,


salt, firstHash)) {
Console.WriteLine("Not matching test failed.");
} else {
Console.WriteLine("Not matching test OK.");
}

// 3 Timer test
Stopwatch sw = new Stopwatch();
int numTests = 200;
sw.Start();
for (int i = 0; i < numTests; ++i) {
password1[0] = (byte)i;

firstHash = HashStuff.CalculateHash(new SHA256Managed(),
25000, password1, salt);


} // end for
sw.Stop();
Console.WriteLine("{0} tests took {1} mSec.", numTests,
sw.ElapsedMilliseconds);
Console.WriteLine("Average {0} seconds per hash.",
sw.ElapsedMilliseconds / (numTests * 1000.0));

// Keep console on screen
Console.Write("Press [Enter] to continue... ");
Console.ReadLine();

} // end Main()

} // end class HashTest

public sealed class HashStuff : IDisposable {

private bool disposed = false;


private static RNGCryptoServiceProvider rng = new
RNGCryptoServiceProvider();

private HashStuff() { } // Prevent instantiation

public static byte[] GenerateSalt(Int32 size) {
byte[] salt = new byte[size];
rng.GetBytes(salt);
return salt;
}

public static byte[] CalculateHash(HashAlgorithm algorithm, Int32
repeats, byte[] data, byte[] salt) {


// Check parameters for nulls and salt size here.
byte[] hash;

using (algorithm) {
hash = algorithm.ComputeHash(Concat(data, salt));
for (int i = 0; i < repeats; ++i) {
hash = algorithm.ComputeHash(Concat(hash, salt));
}
} // end using

// Clear data
for (Int32 j = 0; j < data.Length; ++j) {
data[j] = 0;
}

return hash;
}

public static Boolean CheckHash(HashAlgorithm algorithm, Int32
repeats, byte[] data, byte[] salt, byte[] expected) {

byte[] actual = CalculateHash(algorithm, repeats, data, salt);


for (int i = 0; i < actual.Length; ++i) {
if (actual[i] != expected[i]) { return false; }
}
return true;

}

private static byte[] Concat(byte[] lhs, byte[] rhs) {
byte[] result = new byte[lhs.Length + rhs.Length];
Array.Copy(lhs, result, lhs.Length);
Array.Copy(rhs, 0, result, lhs.Length, rhs.Length);
return result;
}

// Dispose


public void Dispose() {
this.Dispose();
GC.SuppressFinalize(this);
} // Dispose

public void Dispose(bool disposing) {

if (disposed)
return;

if (disposing) {
// Free any managed resources
}

// Free any unmanaged resources
disposed = true;
}

} // end class HashStuff
}

rossum

unread,
Sep 15, 2009, 5:02:16 PM9/15/09
to
On Tue, 15 Sep 2009 10:11:04 -0700 (PDT), shapper <mdm...@gmail.com>
wrote:

>Hello,


>
>I added a few things to your code:
>
>I defined HashAlgorithm, Repeats and SaltSize as inputs.
>
>I didn't define them as properties since on your code you didn't use
>any property and have the class sealed.
>I am not sure if there is a security reason for this or something
>else ...

Sealing is not required, but if you have no reason to subclass your
class it prevents an attacker writing their own subclass and inserting
it into production code:

@Override
public byte[] CalculateHash(byte[] password, byte[] salt) {
copy_to_my_secret_data_capture_file(username, password);
return super.CalculateHash(password, salt);
}


>
>I defined these parameters so I can use different configurations in
>different projects if I need it.
>
>I also cleanup the data input (password) on CalculateHash before I
>return the hash.

Good.

>
>And I tried to implement IDisposable but I am not exactly sure how to
>do it and how to use it correctly.
>I know it has to do with disposing managed and unmanaged resources but
>I am not sure how to integrate it in this class.

I find it easier to use IDisposable on an instance of a class, rather
than on a static class.

Either

HashStuff hs = new HashStuff(SHA256, 25000);
try {
// code using hs
} finally {
hs.Dispose();
}

or else

using (HashStuff hs = new HashStuff(SHA256, 25000)) {
// Code using hs
}

In the second case, the 'using' statement will automatically call
Dispose() for you.

That would mean removing 'static' from the declaration of HashStuff
and from some of the methods and making the required changes in the
calling code.

Since you are allowing different values of the hash algorithm and of
repeats then you probably need to make CalculateHash() and CheckHash()
non-static as they use those parameters. GenerateSalt() can remain
static as it does not use any of the parameters that vary between
instances.

I would suggest that the hash algorithm and repeats parameters are
fixed at construction time, otherwise there will be the possibility of
changing one or both parameters between calls to GenerateHash() and
CheckHash(). That is a possible source of errors.

Try something like:

public sealed class HashStuff : IDisposable {

private bool disposed = false;

private static RNGCryptoServiceProvider rng = new
RNGCryptoServiceProvider();

private static HASHAlgorithm DEFAULT_HASH = new SHA256Managed();
private final HashAlgorithm hashAlgo;

private static int DEFAULT_REPEATS = 10000;
private final int repeats;

public HashStuff(HashAlgorithm algo, int reps)
// Check for null algo
// Check for zero or negative reps
hashAlgo = algo;
repeats = reps;
}

public HashStuff(int reps) {
this(DEFAULT_HASH, reps);
}

public HashStuff(HashAlgorithm algo) {
this(algo, DEFAULT_REPEATS);
}

public HashStuff() {
this(DEFAULT_HASH, DEFAULT_REPEATS);
}

// rest of HashStuff class

} // end class

Looking at the code you posted, there is are issues with your
implementation of Dispose(), you have made a copying mistake and do
not actually Dispose of anything.

> // Dispose
> public void Dispose() {
> this.Dispose();

What you have written will give you an infinite recursion, it should
be:

this.Dispose( true );

which calls the version of Dispose with a boolean parameter.

> GC.SuppressFinalize(this);
> } // Dispose
>
> public void Dispose(bool disposing) {
>
> if (disposed)
> return;
>
> if (disposing) {
> // Free any managed resources

At this point you need to actually free your managed resources:

if (hashAlgo != null) {
hashAlgo.Dispose();
}

> }
>
> // Free any unmanaged resources

I am not sure about the non-managed versions of the Hash algorithms,
it is possible that the non-managed versions use unmanaged resources.
You will have to investigate further.

This is an example of allowing options introducing complications.
Reducing options simplifies code. Simpler code is less error prone
and easier to show to be correct.

> disposed = true;
> }

rossum

Peter Duniho

unread,
Sep 15, 2009, 5:44:23 PM9/15/09
to
On Tue, 15 Sep 2009 14:02:16 -0700, rossum <ross...@coldmail.com> wrote:

> Sealing is not required, but if you have no reason to subclass your
> class it prevents an attacker writing their own subclass and inserting
> it into production code:
>
> @Override
> public byte[] CalculateHash(byte[] password, byte[] salt) {
> copy_to_my_secret_data_capture_file(username, password);
> return super.CalculateHash(password, salt);
> }

> [...]

I'll suggest that sealing a class to prevent such an attack is pointless.
The "sealed" keyword is useful as a maintenance construct, to prevent
certain kinds of uses of one's code so as to avoid bugs.

But if you've got an _attacker_, trying to co-opt some class, the "sealed"
keyword isn't going to prevent sub-classing or otherwise reusing the base
class. They can just hack around it in a variety of ways.

Pete

shapper

unread,
Sep 16, 2009, 10:42:33 AM9/16/09
to
On Sep 15, 10:44 pm, "Peter Duniho" <no.peted.s...@no.nwlink.spam.com>
wrote:

Hello,

I recreated the class with the last advices posted, gave it a short
name Sash, and I got a few problems:

1) When testing I get an error in CalculateHash where algorithm is
null.
This is strange because just before I call GenerateSash the same
way and it works fine.

2) On disposable I get an error:
// Error: 'System.Security.Cryptography.HashAlgorithm.Dispose
(bool)' is inaccessible due to its protection level
// if (algorithm != null) algorithm.Dispose();

Why is this?
I tried to remove readonly from algorithm field but it didn't
solved it.

3) Rossum, when you used "private final ..." I think that was a
confusing from Java, correct?
If I am not wrong the equivalent for fields is "private
readonly" ... This is what I used but I am not sure.

Here is the entire code I am using including some comments and
testing:

public class Program {

public static void Main(String[] args) {

Sash sash = new Sash(new SHA256Managed(), 25000);
try {

// Test matching
Byte[] password1 = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7 };
Byte[] salt = sash.GenerateSalt(64);
Byte[] firstHash = sash.CalculateHash(password1, salt);
Byte[] password2 = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7 };
if (sash.CheckHash(password2, salt, firstHash)) {


Console.WriteLine("Matching test OK");
} else {
Console.WriteLine("Matching test failed.");
}

// Test not matching


password1 = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7 };

salt = sash.GenerateSalt(64);
firstHash = sash.CalculateHash(password1, salt);


password2 = new byte[] { 1, 1, 2, 3, 4, 5, 6, 7 };

if (sash.CheckHash(password2, salt, firstHash)) {


Console.WriteLine("Not matching test failed.");
} else {
Console.WriteLine("Not matching test OK.");
}

// Timer test


Stopwatch sw = new Stopwatch();
int numTests = 200;
sw.Start();
for (int i = 0; i < numTests; ++i) {
password1[0] = (byte)i;

firstHash = sash.CalculateHash(password1, salt);
} // end for
sw.Stop();

Console.WriteLine("{0} tests took {1} mSec.", numTests,
sw.ElapsedMilliseconds);
Console.WriteLine("Average {0} seconds per hash.",
sw.ElapsedMilliseconds / (numTests * 1000.0));

Console.Write("Press [Enter] to continue... ");
Console.ReadLine();

} finally {
sash.Dispose();
}

} // end Main()

} // end class HashTest

// Sash
/// <summary>
/// Implements salted hash for managed has algorithms
/// </summary>
public sealed class Sash : IDisposable {

#region Fields

private Boolean disposed = false;
private readonly Int32 repeats;
private readonly HashAlgorithm algorithm;

private static Int32 defaultRepeats = 10000;
private static HashAlgorithm defaultAlgorithm = new SHA256Managed
();


private static RNGCryptoServiceProvider rng = new
RNGCryptoServiceProvider();

#endregion // Fields

#region Constructors

// Sash
/// <summary>
/// Initialize sash with custom has algorithm and repeats
/// </summary>
public Sash(HashAlgorithm algorithm, Int32 repeats) {

// Check parameters
if (algorithm == null) throw new ArgumentNullException
("algorithm", "Hash algorithm cannot be null");
if (repeats <= 0) throw new ArgumentOutOfRangeException
("repeats", "Repeats must be positive");

// Define fields
algorithm = algorithm;
repeats = repeats;

} // Sash

// Sash
/// <summary>
/// Initialize sash with defaul algorithm, SHA256MAnaged, and
custom repeats value
/// </summary>
public Sash(Int32 repeats) : this(defaultAlgorithm, repeats)
{
} // Sash

// Sash
/// <summary>
/// Initialize sash with custom algorithm and default repeats
equal to 10000
/// </summary>
public Sash(HashAlgorithm algorithm) : this(algorithm,
defaultRepeats) {
} // Sash

// Sash
/// <summary>
/// Initialize sash with defaul algorithm, SHA256MAnaged, and
default repeats equal to 10000
/// </summary>
public Sash() : this(defaultAlgorithm, defaultRepeats) {
} // Sash

#endregion // Constructors

#region Methods

// GenerateSalt
/// <summary>
/// Generate random salt
/// </summary>
public Byte[] GenerateSalt(Int32 size) {
Byte[] salt = new Byte[size];
rng.GetBytes(salt);
return salt;
} // GenerateSalt

// CalculateHash
/// <summary>
/// Calculate hash from a hash algorithm and salt
/// </summary>
public Byte[] CalculateHash(Byte[] data, Byte[] salt) {

// Check parameters
if (data == null) throw new ArgumentNullException("data", "Data
cannot be null");
if (data.Length == 0) throw new ArgumentOutOfRangeException
("data", "Data cannot be empty");
if (salt == null) throw new ArgumentNullException("salt", "Salt
cannot be null");
if (salt.Length == 0) throw new ArgumentOutOfRangeException
("salt", "Salt cannot be empty");

// Calculate hash
Byte[] hash;


using (algorithm) {
hash = algorithm.ComputeHash(Concat(data, salt));

for (Int32 i = 0; i < repeats; ++i) {
hash = algorithm.ComputeHash(Concat(hash, salt));
}
}

// Clear data


for (Int32 j = 0; j < data.Length; ++j) {
data[j] = 0;
}

return hash;

} // CalculateHash

// CheckHash
/// <summary>
/// Check hash against expected
/// </summary>
public Boolean CheckHash(Byte[] data, Byte[] salt, Byte[]
expected) {

// Check parameters
if (data == null) throw new ArgumentNullException("data", "Data
cannot be null");
if (data.Length == 0) throw new ArgumentOutOfRangeException
("data", "Data cannot be empty");
if (salt == null) throw new ArgumentNullException("salt", "Salt
cannot be null");
if (salt.Length == 0) throw new ArgumentOutOfRangeException
("salt", "Salt cannot be empty");
if (expected == null) throw new ArgumentNullException
("expected", "Expected cannot be null");
if (expected.Length == 0) throw new ArgumentOutOfRangeException
("expected", "Expected cannot be empty");

// CalculateHash
Byte[] actual = CalculateHash(data, salt);
for (Int32 i = 0; i < actual.Length; ++i) {


if (actual[i] != expected[i]) { return false; }
}
return true;

} // CheckHash

// Dispose
/// <summary>
/// Dispose resources
/// </summary>
public void Dispose() {
this.Dispose(true);
GC.SuppressFinalize(this);
} // Dispose

// Dispose
/// <summary>
/// Dispose resources
/// </summary>
public void Dispose(Boolean disposing) {

// Already disposed
if (disposed) return;

// Dispose managed resources
if (disposing) {

// Error: 'System.Security.Cryptography.HashAlgorithm.Dispose
(bool)' is inaccessible due to its protection level
// if (algorithm != null) algorithm.Dispose();
}

// Dispose unmanaged resources

// Redefine disposed
disposed = true;

} // Dispose

#region Private

// Concat
/// <summary>
/// Concat two byte arrays
/// </summary>
private Byte[] Concat(byte[] lhs, byte[] rhs) {
Byte[] result = new Byte[lhs.Length + rhs.Length];


Array.Copy(lhs, result, lhs.Length);
Array.Copy(rhs, 0, result, lhs.Length, rhs.Length);
return result;

} // Concat

#endregion // Private

#endregion // Methods

} // Sash

Thanks,
Miguel

rossum

unread,
Sep 16, 2009, 12:02:49 PM9/16/09
to
On Wed, 16 Sep 2009 07:42:33 -0700 (PDT), shapper <mdm...@gmail.com>
wrote:

>Hello,


>
>I recreated the class with the last advices posted, gave it a short
>name Sash, and I got a few problems:
>
>1) When testing I get an error in CalculateHash where algorithm is
>null.
> This is strange because just before I call GenerateSash the same
>way and it works fine.

Inside GenerateHash() the 'using' statement will call Dispose() which
could well leave the algorithm variable null, or at least in an
inoperable state. If you look back at my code you will see that every
time I enter the 'using' statement I new the hash algorithm variable.
You either need to remove the 'using' or to re-new the variable.

>
>2) On disposable I get an error:
> // Error: 'System.Security.Cryptography.HashAlgorithm.Dispose
>(bool)' is inaccessible due to its protection level
> // if (algorithm != null) algorithm.Dispose();
>
> Why is this?
> I tried to remove readonly from algorithm field but it didn't
>solved it.

Looking at the documentation, the HashAlgorithm.Dispose(bool) method
is protected. That seems to be where the problem lies.

>
>3) Rossum, when you used "private final ..." I think that was a
>confusing from Java, correct?

Yes, thanks for catching my error. I do more Java programming than C#
at the moment.

> If I am not wrong the equivalent for fields is "private
>readonly" ... This is what I used but I am not sure.

You are correct.

>
>Here is the entire code I am using including some comments and
>testing:
>
> public class Program {
>
> public static void Main(String[] args) {
>

[snip test code]


>
> } // end Main()
>
> } // end class HashTest
>
>
>
> // Sash
> /// <summary>
> /// Implements salted hash for managed has algorithms

Typo. has -> hash

> /// </summary>
> public sealed class Sash : IDisposable {
>
> #region Fields
>
> private Boolean disposed = false;
> private readonly Int32 repeats;
> private readonly HashAlgorithm algorithm;
>
> private static Int32 defaultRepeats = 10000;
> private static HashAlgorithm defaultAlgorithm = new SHA256Managed
>();
> private static RNGCryptoServiceProvider rng = new
>RNGCryptoServiceProvider();
>
> #endregion // Fields
>
> #region Constructors
>
> // Sash
> /// <summary>
> /// Initialize sash with custom has algorithm and repeats

Typo. has -> hash

> /// </summary>
> public Sash(HashAlgorithm algorithm, Int32 repeats) {
>
> // Check parameters
> if (algorithm == null) throw new ArgumentNullException
>("algorithm", "Hash algorithm cannot be null");
> if (repeats <= 0) throw new ArgumentOutOfRangeException
>("repeats", "Repeats must be positive");
>
> // Define fields
> algorithm = algorithm;
> repeats = repeats;

This is confusing. Better to write:

this.algorithm = algorithm;
this.repeats = repeats;

alternatively just rename the parameters to algo and reps or whatever.
Either way you need to make some distinction in code between the
method parameter and the class member variable.

>
> } // Sash
>
> // Sash
> /// <summary>
> /// Initialize sash with defaul algorithm, SHA256MAnaged, and

Typo. defaul -> default

>custom repeats value
> /// </summary>
> public Sash(Int32 repeats) : this(defaultAlgorithm, repeats)
>{
> } // Sash
>
> // Sash
> /// <summary>
> /// Initialize sash with custom algorithm and default repeats
>equal to 10000
> /// </summary>
> public Sash(HashAlgorithm algorithm) : this(algorithm,
>defaultRepeats) {
> } // Sash
>
> // Sash
> /// <summary>
> /// Initialize sash with defaul algorithm, SHA256MAnaged, and

Typo. defaul -> default
Typo. SHA256MAnaged -> SHA256Managed

I talk about this using statement above.

Better to check that expected is the right length for the hash
algorithm you are using:

if (expected.Length != algorithm.HashSize / 8) throw new ...

See comments above. You probably need to check the documentation for
HashAlgorithm.

rossum

shapper

unread,
Sep 16, 2009, 2:25:12 PM9/16/09
to
On Sep 16, 5:02 pm, rossum <rossu...@coldmail.com> wrote:
> See comments above.  You probably need to check the documentation for
> HashAlgorithm.

Hi,

I made the corrections, removed using and went to HashAlgorithm
documentation on MSDN:
http://msdn.microsoft.com/en-us/library/system.security.cryptography.hashalgorithm_members%28VS.71%29.aspx

If I am not wrong it should be used the Clear method instead of the
protected Disposed method:
Clear: Releases all resources used by the HashAlgorithm.
Dispose: Releases the unmanaged resources used by the HashAlgorithm
and optionally releases the managed resources.

I ran the code and got the following results:


Matching test OK
Not matching test OK

200 tests took 123145 mSec
Average 0.615725 seconds per hash

I just kept the 250 000 repeats as you used.
But in this project I will use a lower value to not be so heavy to the
CPU and because there is not really important data to protect but in
future projects this will be really useful.

Any advice for the length I should keep on the database for the
Password and Salt columns? I will make both varbinary.

This is the working code:

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text;
using System.Security.Cryptography;

namespace SashImplementation {

public class Program {

public static void Main(String[] args) {

Sash sash = new Sash(new SHA256Managed(), 250000);
try {

// Keep console on screen


Console.Write("Press [Enter] to continue... ");
Console.ReadLine();


} finally {
sash.Dispose();
}

} // end Main()

} // end class HashTest

// Sash
/// <summary>
/// Implements salted hash for managed has algorithms

/// </summary>
public sealed class Sash : IDisposable {

#region Fields

private Boolean disposed = false;
private readonly Int32 repeats;
private readonly HashAlgorithm algorithm;

private static Int32 defaultRepeats = 10000;
private static HashAlgorithm defaultAlgorithm = new SHA256Managed
();
private static RNGCryptoServiceProvider rng = new
RNGCryptoServiceProvider();

#endregion // Fields

#region Constructors

// Sash
/// <summary>
/// Initialize sash with custom hash algorithm and repeats


/// </summary>
public Sash(HashAlgorithm algorithm, Int32 repeats) {

// Check parameters
if (algorithm == null) throw new ArgumentNullException
("algorithm", "Hash algorithm cannot be null");
if (repeats <= 0) throw new ArgumentOutOfRangeException
("repeats", "Repeats must be positive");

// Define fields


this.algorithm = algorithm;
this.repeats = repeats;

} // Sash

// Sash
/// <summary>
/// Initialize sash with default algorithm, SHA256Managed, and


custom repeats value
/// </summary>
public Sash(Int32 repeats)
: this(defaultAlgorithm, repeats) {
} // Sash

// Sash
/// <summary>
/// Initialize sash with custom algorithm and default repeats
equal to 10000
/// </summary>
public Sash(HashAlgorithm algorithm)
: this(algorithm, defaultRepeats) {
} // Sash

// Sash
/// <summary>
/// Initialize sash with defaul algorithm, SHA256Managed, and


default repeats equal to 10000
/// </summary>

#endregion // Constructors

#region Methods

return hash;

} // CalculateHash

if (expected.Length != algorithm.HashSize / 8) throw new

ArgumentOutOfRangeException("expected", "Expected length should be one
eight of algorithm hash size");

// CalculateHash
Byte[] actual = CalculateHash(data, salt);
for (Int32 i = 0; i < actual.Length; ++i) {
if (actual[i] != expected[i]) { return false; }
}
return true;

} // CheckHash

// Dispose
/// <summary>
/// Dispose resources
/// </summary>
public void Dispose() {
this.Dispose(true);
GC.SuppressFinalize(this);
} // Dispose

// Dispose
/// <summary>
/// Dispose resources
/// </summary>
public void Dispose(Boolean disposing) {

// Already disposed
if (disposed) return;

// Dispose managed resources
if (disposing) {

if (algorithm != null) algorithm.Clear();
}

// Dispose unmanaged resources

// Redefine disposed
disposed = true;

} // Dispose

#region Private

// Concat
/// <summary>
/// Concat two byte arrays
/// </summary>
private Byte[] Concat(byte[] lhs, byte[] rhs) {
Byte[] result = new Byte[lhs.Length + rhs.Length];
Array.Copy(lhs, result, lhs.Length);
Array.Copy(rhs, 0, result, lhs.Length, rhs.Length);
return result;
} // Concat

#endregion // Private

#endregion // Methods

} // Sash

} // SashImplementation

rossum

unread,
Sep 16, 2009, 3:19:55 PM9/16/09
to
On Wed, 16 Sep 2009 11:25:12 -0700 (PDT), shapper <mdm...@gmail.com>
wrote:

>On Sep 16, 5:02 pm, rossum <rossu...@coldmail.com> wrote:


>> See comments above.  You probably need to check the documentation for
>> HashAlgorithm.
>
>Hi,
>
>I made the corrections, removed using and went to HashAlgorithm
>documentation on MSDN:
>http://msdn.microsoft.com/en-us/library/system.security.cryptography.hashalgorithm_members%28VS.71%29.aspx
>
>If I am not wrong it should be used the Clear method instead of the
>protected Disposed method:
>Clear: Releases all resources used by the HashAlgorithm.

That would explain the problems with trying to call Dispose directly.

>Dispose: Releases the unmanaged resources used by the HashAlgorithm
>and optionally releases the managed resources.
>
>I ran the code and got the following results:
>Matching test OK
>Not matching test OK
>200 tests took 123145 mSec
>Average 0.615725 seconds per hash
>
>I just kept the 250 000 repeats as you used.
>But in this project I will use a lower value to not be so heavy to the
>CPU and because there is not really important data to protect but in
>future projects this will be really useful.

Yes, repeats needs to be changed each time the code is moved to a
new/upgraded machine.

>
>Any advice for the length I should keep on the database for the
>Password and Salt columns? I will make both varbinary.

Password lengths should be specified in either company policy or user
instructions. Six or eight characters is a minimum with a maximum of
fifteen or twenty for passwords or 100 for passphrases. You may also
want to enforce the use of mixed case, digits and non-alphanum
characters.

I do think that you should stay with a fixed length salt. If not then
the minumum length is 8 bytes (64 bits) and the maximum 32 bytes (256
bits)

>
>This is the working code:

[snip code]

rossum

Arne Vajhøj

unread,
Sep 16, 2009, 10:13:44 PM9/16/09
to
rossum wrote:
> On Mon, 14 Sep 2009 22:23:24 -0400, Arne Vajhøj <ar...@vajhoej.dk>
> wrote:
>> rossum wrote:
>>> The value of REPEATS needs to be set so that the hash calculation
>>> takes about 0.25 second on whatever machine you are using.
>> That comes as a cost.
>>
>> 4 logins (attempts) per second cost a CPU core.
> Correct, security costs. If you want the security then you pay the
> cost.

True.

But:
- using SHA-256 instead of MD5
- using a per user random salt
- enforce long passwords
- enforce more possible characters in passwords
all gives a lot increase security for a very small cost (you
pay 10% more in CPU usage and brute force becomes 10000 times
more expensive).

The repeated hashing is a different type of change because
it cost a lot (10000 times more CPU usage and brute force
becomes 10000 times more expensive).

> To quote Bruce Schneier: "There are already enough insecure
> fast systems, we don't need another one." How much will it cost if an
> attacker gets access to the passwords? How much will it cost to add
> some more CPU to the server farm? Those are questions for the OP to
> answer. If a court case ensues after loss of data then it is
> important to be able to show that best security practice was being
> used.
>
>> For high volume it costs. And besides the increase in DoS
>> vulnerability may be more important than the added protection
>> against exploiting a stolen database.
> That is the sort of thing that the OP has to assess. We do not know
> that value of the data protected by the passwords.

One important note is that the original poster does not know
the value of the data being protected either.

If someone gets away with the hashed passwords from the database
they will most likely also have gotten away with all the other
data in the database.

What good hashing protects against is the hackers finding the
plain text password and use it at other sites where the users
has used the same password.

The original poster has no way of knowing whether the users
are smart enough to only use the same password at one site
or if they just use this password at unimportant web sites or
if they use the same password at their bank and credit card
company.

Arne


Arne Vajhøj

unread,
Sep 16, 2009, 10:16:03 PM9/16/09
to
shapper wrote:
> On Sep 15, 11:56 am, rossum <rossu...@coldmail.com> wrote:
>> On Mon, 14 Sep 2009 22:23:24 -0400, Arne Vajh�j <a...@vajhoej.dk>

>> wrote:
>>> rossum wrote:
>>>> The value of REPEATS needs to be set so that the hash calculation
>>>> takes about 0.25 second on whatever machine you are using.
>>> That comes as a cost.
>>> 4 logins (attempts) per second cost a CPU core.
>> Correct, security costs. If you want the security then you pay the
>> cost. To quote Bruce Schneier: "There are already enough insecure
>> fast systems, we don't need another one." How much will it cost if an
>> attacker gets access to the passwords? How much will it cost to add
>> some more CPU to the server farm? Those are questions for the OP to
>> answer. If a court case ensues after loss of data then it is
>> important to be able to show that best security practice was being
>> used.
>>
>>> For high volume it costs. And besides the increase in DoS
>>> vulnerability may be more important than the added protection
>>> against exploiting a stolen database.
>> That is the sort of thing that the OP has to assess. We do not know
>> that value of the data protected by the passwords.
>
> In this case the most important data I have for each user its their
> email ... In this case security is not a "big issue" ...

The big issue is if your users use the same password at their
internet banking as at your site.

> But I would like to have a secure and correct way to do this ...

That is good.

> I can then, from project to project, change the REPEATS from 25 000 to
> 10 000 or just 1 according to the level of security and processor cost
> for each situation ...

You could.

Or you could try and enforce long random passwords which has a
similar effect but are much cheaper.

Note though that users tend to dislike such attempts.

Arne

Arne Vajhøj

unread,
Sep 16, 2009, 10:17:25 PM9/16/09
to

Absolutely.

Decompile with reflector, delete sealed keyword and recompile takes
just seconds.

Arne

Jesse Houwing

unread,
Sep 16, 2009, 10:31:03 PM9/16/09
to
* rossum wrote, On 16-9-2009 21:19:
> On Wed, 16 Sep 2009 11:25:12 -0700 (PDT), shapper<mdm...@gmail.com>
> wrote:
>
>> On Sep 16, 5:02 pm, rossum<rossu...@coldmail.com> wrote:
>>> See comments above. You probably need to check the documentation for
>>> HashAlgorithm.
>>
>> Hi,
>>
>> I made the corrections, removed using and went to HashAlgorithm
>> documentation on MSDN:
>> http://msdn.microsoft.com/en-us/library/system.security.cryptography.hashalgorithm_members%28VS.71%29.aspx
>>
>> If I am not wrong it should be used the Clear method instead of the
>> protected Disposed method:
>> Clear: Releases all resources used by the HashAlgorithm.
> That would explain the problems with trying to call Dispose directly.

You'll probably see that Dispose calls Clear as well. But dispose is
probably implemented explicitly, which means that in order to call
Dispose you first need to cast the object to IDisposable. It's nice to
know this in case you need to clean up a whole list of Disposable
objects. I sometimes tend to pass them in a foreach loop.

The proper syntax would be:

HashAlgorithm x
try
{
x = new ...();
..
..
..
}
finally
{
((IDisposable)x).Dispose(); // Same as x.Clear();
}

You whould also be able to use

using(HashAlgorithm x = new ...())
{
..
..
..
}

Which will clean up after itself.

>> Dispose: Releases the unmanaged resources used by the HashAlgorithm
>> and optionally releases the managed resources.
>>
>> I ran the code and got the following results:
>> Matching test OK
>> Not matching test OK
>> 200 tests took 123145 mSec
>> Average 0.615725 seconds per hash
>>
>> I just kept the 250 000 repeats as you used.
>> But in this project I will use a lower value to not be so heavy to the
>> CPU and because there is not really important data to protect but in
>> future projects this will be really useful.
> Yes, repeats needs to be changed each time the code is moved to a
> new/upgraded machine.

Can't you just start a StopWatch and return after x time has elapsed?
That would make it suitable for any machine adding little overhead.

What I don't understand is why Thread.Sleep(.5sec) won't suffice...

Is this to make sure a really random hash is created? Or is it just to
make sure that you cannot compute too many hashes at the same time? If
the latter, shouldn't this method by synced or locked in any way? You
could just start 80 threads/requests in parallel and much of the use of
this delay is voided. If any of the threads returns with a response, you
know you've found the right combination.

Do you have a link that explains why you should do this? And it makes me
wonder why the HashProvider hasn't got a parameter that says, keep
hashing till x time has elapsed if it's standard practice.
Just trying to learn here :).

>> Any advice for the length I should keep on the database for the
>> Password and Salt columns? I will make both varbinary.
> Password lengths should be specified in either company policy or user
> instructions. Six or eight characters is a minimum with a maximum of
> fifteen or twenty for passwords or 100 for passphrases. You may also
> want to enforce the use of mixed case, digits and non-alphanum
> characters.

The length of the salt can be chosen by yourself. (max 32 bytes as
Rossum says. The length of the resulting Hashed password depends on the
algorithm used. I'd just create the ma length of the longest posible
hash and pad any smaller hashes with 'blank spaces'.

> I do think that you should stay with a fixed length salt. If not then
> the minumum length is 8 bytes (64 bits) and the maximum 32 bytes (256
> bits)


--
Jesse Houwing
jesse.houwing at sogeti.nl

shapper

unread,
Sep 17, 2009, 7:29:30 AM9/17/09
to
On Sep 17, 3:16 am, Arne Vajhøj <a...@vajhoej.dk> wrote:
> The big issue is if your users use the same password at their
> internet banking as at your site.

That is a good point. I haven't think about it ...

>
> Or you could try and enforce long random passwords which has a
> similar effect but are much cheaper.
> Note though that users tend to dislike such attempts.

Enforce a random password to the user? Even I don't like it :-)

I think it would be more suitable to force the user to have a password
with length bigger than 10 characters and in some cases to have
numbers, letters a punctuation mark like a dot or something ...

shapper

unread,
Sep 17, 2009, 7:32:37 AM9/17/09
to
On Sep 16, 8:19 pm, rossum <rossu...@coldmail.com> wrote:
> Password lengths should be specified in either company policy or user
> instructions.  Six or eight characters is a minimum with a maximum of
> fifteen or twenty for passwords or 100 for passphrases.  You may also
> want to enforce the use of mixed case, digits and non-alphanum
> characters.

When you say passphrases you mean that when using passwords a space
shouldn't be allowed?
I am just wondering if I should include that in validation of the
form.

> I do think that you should stay with a fixed length salt.  If not then
> the minumum length is 8 bytes (64 bits) and the maximum 32 bytes (256
> bits)

Yes yes I will keep salt a fixed length. I was just wondering the
minimum and maximum size of passwords and salt also to be able to
define a proper length on the SQL table columns.

Thank You,
Miguel

rossum

unread,
Sep 17, 2009, 10:30:52 AM9/17/09
to
On Thu, 17 Sep 2009 04:32:37 -0700 (PDT), shapper <mdm...@gmail.com>
wrote:

>On Sep 16, 8:19 pm, rossum <rossu...@coldmail.com> wrote:


>> Password lengths should be specified in either company policy or user
>> instructions.  Six or eight characters is a minimum with a maximum of
>> fifteen or twenty for passwords or 100 for passphrases.  You may also
>> want to enforce the use of mixed case, digits and non-alphanum
>> characters.
>
>When you say passphrases you mean that when using passwords a space
>shouldn't be allowed?

Yes, a password has no spaces while a passphrase may do so.
Passphrases are usually longer, which should make them more secure.

>I am just wondering if I should include that in validation of the
>form.

If there are rules for a correctly formed password/phrase then those
rules should be enforced.

rossum

Scott Seligman

unread,
Sep 17, 2009, 12:39:26 PM9/17/09
to
rossum <ross...@coldmail.com> wrote:
>Yes, a password has no spaces while a passphrase may do so.
>Passphrases are usually longer, which should make them more secure.

Why shouldn't a password have spaces?

For that matter, why have limit on password lengths?

--
--------- Scott Seligman <scott at <firstname> and michelle dot net> ---------
There are fewer great satisfactions than that of self.
-- Calhoun in Star Trek: New Frontier: Being Human by Peter David

shapper

unread,
Sep 17, 2009, 2:42:52 PM9/17/09
to
On Sep 17, 5:39 pm, "Scott Seligman" <selig...@example.com> wrote:
> Why shouldn't a password have spaces?

That's what I thought ... For me a password is a password.
With spaces, numbers, punctuation, etc ... I can restrict the the
allowed characters but I always saw it as a password.

> For that matter, why have limit on password lengths?

Usually I set a minimum limit and for maximum minimum usually set a
reasonable value to store in a table column without having a lot of
records with a lot of empty space ...
... I usually go for 40.

Scott Seligman

unread,
Sep 17, 2009, 3:42:56 PM9/17/09
to

While I do think there are arguments for sanity-check type upper limits
(mostly having to do with moving the passwords around, I don't want to
try and hash a 1gb password, for instance), you shouldn't be storing
them in a database at all, only the hash of passwords.

--
--------- Scott Seligman <scott at <firstname> and michelle dot net> ---------

He had to get up to run some more. He told his muscles to do so. They
told him they had the night off.
-- Deadly Relations: Bester Ascendant by J. Gregory Keyes

rossum

unread,
Sep 17, 2009, 4:41:01 PM9/17/09
to
On 17 Sep 2009 09:39:26 -0700, "Scott Seligman" <seli...@example.com>
wrote:

>rossum <ross...@coldmail.com> wrote:
>>Yes, a password has no spaces while a passphrase may do so.
>>Passphrases are usually longer, which should make them more secure.
>
>Why shouldn't a password have spaces?
>
>For that matter, why have limit on password lengths?

Human memory. It is easier to memorise a long phrase than a long
string of nonsense.

- My name is Ozymandias, king of kings. Look on my works ye mighty
and despair.

- Qwglakjnf73k56gboDYkasnfDovmt;'sDaksdflkp6823fvYEDV94u6jflajnflj
qwmcljncxl.m

One possible technique is to turn the first into:

- MniOkokLomwymad

rossum

Scott Seligman

unread,
Sep 17, 2009, 8:20:51 PM9/17/09
to
rossum <ross...@coldmail.com> wrote:
>On 17 Sep 2009 09:39:26 -0700, "Scott Seligman" <seli...@example.com>
>wrote:
>
>>rossum <ross...@coldmail.com> wrote:
>>>Yes, a password has no spaces while a passphrase may do so.
>>>Passphrases are usually longer, which should make them more secure.
>>
>>Why shouldn't a password have spaces?
>>
>>For that matter, why have limit on password lengths?
>Human memory. It is easier to memorise a long phrase than a long
>string of nonsense.

Sure, but how does limiting the password length or
preventing the user from entering spaces help?

--
--------- Scott Seligman <scott at <firstname> and michelle dot net> ---------

It's a very sobering feeling to be up in space and realize that one's
safety factor was determined by the lowest bidder on a government
contract. -- Alan Shephard

Arne Vajhøj

unread,
Sep 17, 2009, 10:04:35 PM9/17/09
to
shapper wrote:

> On Sep 17, 3:16 am, Arne Vajh�j <a...@vajhoej.dk> wrote:
>> The big issue is if your users use the same password at their
>> internet banking as at your site.
>
> That is a good point. I haven't think about it ...

It is important.

>> Or you could try and enforce long random passwords which has a
>> similar effect but are much cheaper.
>> Note though that users tend to dislike such attempts.
>
> Enforce a random password to the user? Even I don't like it :-)
>
> I think it would be more suitable to force the user to have a password
> with length bigger than 10 characters and in some cases to have
> numbers, letters a punctuation mark like a dot or something ...

A password of 10 characters is not really long after todays
security standards.

You should try and calculate how many possible
passwords some schemes can generate.

A 12 character long random password from A-Za-z0-9 has
3E21 possible values.

A password of an English word with a 2 digits suffix
has about 1 million possible values.

Arne

Arne Vajhøj

unread,
Sep 17, 2009, 10:09:12 PM9/17/09
to
Scott Seligman wrote:
> rossum <ross...@coldmail.com> wrote:
>> Yes, a password has no spaces while a passphrase may do so.
>> Passphrases are usually longer, which should make them more secure.
>
> Why shouldn't a password have spaces?
>
> For that matter, why have limit on password lengths?

Words do not have spaces - phrases does.

So by adding the option of spaces the password becomes
a passphrase.

It is about English not about software.

Arne

Scott Seligman

unread,
Sep 18, 2009, 12:44:00 AM9/18/09
to
=?ISO-8859-1?Q?Arne_Vajh=F8j?= <ar...@vajhoej.dk> wrote:
>Scott Seligman wrote:
>> rossum <ross...@coldmail.com> wrote:
>>> Yes, a password has no spaces while a passphrase may do so.
>>> Passphrases are usually longer, which should make them more secure.
>>
>> Why shouldn't a password have spaces?
>>
>> For that matter, why have limit on password lengths?
>
>Words do not have spaces - phrases does.
>
>So by adding the option of spaces the password becomes
>a passphrase.

A word does not have a special number, mixed case or numbers.

>It is about English not about software.

Okay, have fun.

--
--------- Scott Seligman <scott at <firstname> and michelle dot net> ---------

When the gods choose to punish us, they merely answer our prayers.
-- Oscar Wilde

Arne Vajhøj

unread,
Sep 20, 2009, 10:34:11 AM9/20/09
to
Scott Seligman wrote:
> =?ISO-8859-1?Q?Arne_Vajh=F8j?= <ar...@vajhoej.dk> wrote:
>> Scott Seligman wrote:
>>> rossum <ross...@coldmail.com> wrote:
>>>> Yes, a password has no spaces while a passphrase may do so.
>>>> Passphrases are usually longer, which should make them more secure.
>>> Why shouldn't a password have spaces?
>>>
>>> For that matter, why have limit on password lengths?
>> Words do not have spaces - phrases does.
>>
>> So by adding the option of spaces the password becomes
>> a passphrase.
>
> A word does not have a special number, mixed case or numbers.

True.

But the distinction is still made between words with no
spaces and phrases with spaces.

Arne

0 new messages