Security risks from trimming a salt

Question

So I recently created a static class for password related methods and had to make a method that generated a secure salt.

Initially I implemented RNGCryptoServiceProvider and filed n bytes into an array, which I converted to base64 and returned.

The issue was that with the output length, after conversion of course it was longer than n (which makes sense).

To fix this I changed the function to the method below, and I was just wondering if by trimming the base64 string any security risks are raised?

/// <summary>
/// Generates a salt for use with the Hash method.
/// </summary>
/// <param name="length">The length of string to generate.</param>
/// <returns>A cryptographically secure random salt.</returns>
public static string GenerateSalt(int length)
{
    // Check the length isn't too short.
    if (length < MIN_LENGTH)
    {
        throw new ArgumentOutOfRangeException("length", "Please increase the salt length to meet the minimum acceptable value of " + MIN_LENGTH + " characters.");
    }

    // Calculate the number of bytes required.
    // https://en.wikipedia.org/wiki/Base64#Padding
    // http://stackoverflow.com/questions/17944/how-to-round-up-the-result-of-integer-division
    int bytelen = ((3 * length) + 4 - 1) / 4;

    // Create our empty salt array.
    byte[] bytes = new byte[bytelen];

    // Where we'll put our generated salt.
    string salt;

    // Generate a random secure salt.
    using (RNGCryptoServiceProvider randcrypto = new RNGCryptoServiceProvider())
    {
        // Fill our array with random bytes.
        randcrypto.GetBytes(bytes);

        // Get a base64 string from the random byte array.
        salt = GetBase64(bytes);
    }

    // Trim the end off only if we need to.
    if (salt.Length > length)
    {
        // Substring is the fastest method to use.
        salt = salt.Substring(0, length);
    }

    // Return the salt.
    return salt;
}

Also as a side question, I was having a quick look around and couldn't actually find what the hash function of the C# implementation of RNGCryptoServiceProvider actually is. Anyone know offhand?

Why would you be passing the salt around as a string? Why is the length of this *string* a critical factory? — Damien_The_Unbeliever, Aug 22 '12 at 08:34
@Damien_The_Unbeliever For when the salt is stored in the database, it is more practical to know what kind of output you will get. — Samuel Parkinson, Aug 22 '12 at 08:37
Virtually every database I've heard of has some mechanism that allows you to store binary data. — Damien_The_Unbeliever, Aug 22 '12 at 08:40
As a side note I think your `Calculate the number of bytes required` line is incorrect. As a guide, 0 => 0, 2 => 1 and 3 => 2 (then 4 => 3, 6 => 4 etc.), but 1 should be meaningless as no input can generate a Base64 string of length 1 (mod 4). — Rawling, Aug 22 '12 at 08:40

score 3 · Answer 1 · 2012-08-22T08:45:14.330

3

Why is the length of the salt so important to you? I wouldn't think that there are any real security implication, since the only real requirement of a salt is that it be random and unguessable.

In other words, go for it.

EDIT: Here is another way of doing it using Linq.

Random random = new Random();
int length = 25; // Whatever length you want
char[] keys = "ABCDEFGHIJKLMNOPQRSTUVWXYZ01234567890!£$%^&*()".ToCharArray(); // whatever chars you want
var salt = Enumerable
    .Range(1, length) // equivalent to the loop bit, for(i.. ) 
    .Select(k => keys[random.Next(0, keys.Length - 1)])  // generate a new random char 
    .Aggregate("", (e, c) => e + c); // join them together into a string

edited Aug 22 '12 at 08:45

answered Aug 22 '12 at 08:31

The main reason was if it was used with an existing database that has a maximum size requirement for the salt (such as 16 char or something small where you might as well max it every time). – Samuel Parkinson Aug 22 '12 at 08:35
1

Fair enough. You could just generate a GUID and grab the first sixteen characters of that! – Aug 22 '12 at 08:36
But then you are limited to 32 chars, might as well make something I can use as a general method. – Samuel Parkinson Aug 22 '12 at 08:38
I disagree with your assertion that the length of a salt is unimportant. The purpose of the salt is to prevent a pre-computation attack. In the extreme scenario where you have a 1-bit salt, the salt forces the attacker to create 2 pre-computed sets of data rather than 1 (twice the space). Similarly, a 1-byte salt would require only 256 pre-computed sets, and so on. Since rainbow tables can trade storage space for computation time, the practical minimum may be larger than you think. – Luke Aug 22 '12 at 15:46
@Luke: I think you're being a bit pedantic. I said that the length of the salt shouldn't concern you and that the requirement of the salt was that it be "random and unguessable". Clearly a 1-bit salt is guessable in, at most, two attempts. I was writing instructions for a human being, not for a computer and thus common sense should prevail! – Aug 28 '12 at 07:21

Guffa · Accepted Answer · 2012-08-22T08:37:29.167

There is no security risk with that way of generating the salt.

The salt doesn't need that level of security at all, it's just there so that rainbow tables can't be used to crack the hash/encryption. The regular Random class would be enough to create a salt.

Example:

/// <summary>
/// Generates a salt for use with the Hash method.
/// </summary>
/// <param name="length">The length of string to generate.</param>
/// <returns>A random salt.</returns>
public static string GenerateSalt(int length) {
    // Check the length isn't too short.
    if (length < MIN_LENGTH) {
        throw new ArgumentOutOfRangeException("length", "Please increase the salt length to meet the minimum acceptable value of " + MIN_LENGTH + " characters.");
    }

    // Where we'll put our generated salt.
    StringBuilder salt = new StringBuilder(length);

    // Fill our string with random characters.
    Random rnd = new Random();
    string chars = "0123456798ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
    for (int i = 0; i < length; i++) {
      salt.Append(chars[rnd.Next(chars.Length)]);
    }

    // Return the salt.
    return salt.ToString();
}

Note: If the function would be used more than once close in time, you would use a single Random object and pass into the function, as Random instances created too close in time will give the same random sequence.

With that issue of using `Random`, that is why I use `RNGCryptoServiceProvider`. Might as well if I have the choice :P — Samuel Parkinson, Aug 22 '12 at 08:39
@Sam: Seeing your comment in the code about `Substring` being the fastest method to truncate the string, consider that the encryption random generator is *much* slower than the `Random` class. — Guffa, Aug 22 '12 at 08:41
Fair point, but my thoughts are not to risk security just to improve speed by a marginal amount. In my view, it's not worth using `Random` certainly in a [static method](http://blogs.msdn.com/b/pfxteam/archive/2009/02/19/9434171.aspx). — Samuel Parkinson, Aug 22 '12 at 08:48

Tom Chantler · Answer 3 · 2012-08-22T09:45:36.110

Just for fun, here is a much faster way of doing it (even though the code doesn't look great). Try cutting and pasting this to see. On my machine it executed in about 1.6s compared to 7.1s. Since I was doing a million iterations in each case, I don't think the execution time is that important!

string msg = "";
int desiredLength = 93; // Length of salt required
Stopwatch watch = new Stopwatch();
watch.Start();
for (int k=0; k<1000000; k++)
{
    double guidsNeeded = Math.Ceiling(desiredLength / 36.0);
    string salt = "";
    for (int i=0; i<guidsNeeded; i++)
    {
       salt += Guid.NewGuid().ToString();
    }
    salt = salt.Substring(0,desiredLength);
}
msg += watch.ElapsedMilliseconds.ToString(); // 1654 ms

watch.Start();
for (int j=0; j<1000000; j++)
{
    GenerateSalt(93);
}
msg += "\r\n" + watch.ElapsedMilliseconds.ToString(); // 7096 ms

This is using Guffa's code for GenerateSalt.

Bit of fun knowing it's safe to trim generated salts :) Though I've never liked how similar guid's can be. — Samuel Parkinson, Aug 22 '12 at 09:45

Security risks from trimming a salt

3 Answers3