Generating (presumably) unique `nonce` fields for Twitter OAuth

Question

This is code that's part of a library (closed source) that I have that generates a unique nonce value for Twitter OAuth. Essentially, it generates a random number, combines it in string form with an extra string separated by a pipe, computes a Sha-1 hash, and then repeats the process if the Sha-1 hash (when converted to Base64) contained any non-alpha-numeric characters, which are subsequently stripped.

/// <summary>
/// Generates a (presumably) unique NONCE string for use in Twitter OAuth requests.
/// </summary>
/// <param name="extra">Extra data to add during generation.</param>
/// <returns>
/// A (presumably) unique NONCE string.
/// </returns>
/// <remarks>
/// While we can assume the generated NONCE strings will be somewhat unique, there is a small possibility that they will not be, and in such cases a new NONCE should be generated.
/// 
/// The generated NONCE values are 32 characters in length, and are generated by a Sha-1 hash and a Random generator, which generates four random numbers to be hashed until the generated string is 32 characters in length.
/// </remarks>
public string GenerateNonce(string extra = "")
{
    string result = "";
    SHA1 sha1 = SHA1.Create();

    Random rand = new Random();

    while (result.Length < 32)
    {
        string[] generatedRandoms = new string[4];

        for (int i = 0; i < 4; i++)
        {
            generatedRandoms[i] = rand.Next().ToString();
        }

        result += Convert.ToBase64String(sha1.ComputeHash(Encoding.ASCII.GetBytes(string.Join("", generatedRandoms) + "|" + extra))).Replace("=", "").Replace("/", "").Replace("+", "");
    }

    return result.Substring(0, 32);
}

The idea is to generate somewhat unique nonce strings for use with Twitter OAuth (et al.).

I use the term "presumably," because, while it's not considered a bug that the nonce strings will not be guaranteed to be unique, collisions are expected to be fairly rare. I do welcome any and all suggestions that would lead to more uniqueness.

Answer 1

If I understand the requirements correctly, Guid.NewGuid().ToString("N") would work for generating a nonce.

A GUID is a 128-bit integer (16 bytes) that can be used across all computers and networks wherever a unique identifier is required. Such an identifier has a very low probability of being duplicated.

The "N" format specifier will return the GUID as 32 (lowercase) hexadecimal digits.

Answer 2

Although @mjolka has provided the way to go, I would like to review the code nevertheless.

public string GenerateNonce(string extra = "")
{
    string result = "";
    SHA1 sha1 = SHA1.Create();

    Random rand = new Random();

    while (result.Length < 32)
    {
        string[] generatedRandoms = new string[4];

        for (int i = 0; i < 4; i++)
        {
            generatedRandoms[i] = rand.Next().ToString();
        }

        result += Convert.ToBase64String(sha1.ComputeHash(Encoding.ASCII.GetBytes(string.Join("", generatedRandoms) + "|" + extra))).Replace("=", "").Replace("/", "").Replace("+", "");
    }

    return result.Substring(0, 32);
}  

There are a few things that bothers me.

1. the creation of the string[] generatedRandoms in a loop and later on using string.join() with an empty string as separator

   This should be replaced using a StringBuilder like so

   public static string GenerateNonce(string extra = "")
   {
       string result = "";
       SHA1 sha1 = SHA1.Create();
   
       Random rand = new Random();
       StringBuilder sb = new StringBuilder(1024);
       while (result.Length < 32)
       {
           sb.Length = 0;
           string[] generatedRandoms = new string[4];
   
           for (int i = 0; i < 4; i++)
           {
               sb.Append(rand.Next());
           }
   
           sb.Append("|")
               .Append(extra);
   
           result += Convert.ToBase64String(sha1.ComputeHash(Encoding.ASCII.GetBytes(sb.ToString()))).Replace("=", "").Replace("/", "").Replace("+", "");
       }
   
       return result.Substring(0, 32);
   }
   

2. this very long line of code where you add to the result

   This could be made pretier by using multiple lines like so

   result += Convert.ToBase64String(
       sha1.ComputeHash(Encoding.ASCII.GetBytes(sb.ToString()))
       ).Replace("=", "")
        .Replace("/", "")
        .Replace("+", "");  
   

   in this way one wouldn't need to scroll to the right that much

3. the usage of Replace to replace a = from the string

   For a base64 string the only place a = can occur will be at the end of the string. So instead of using Replace you could do a simple TrimEnd which is a lot faster.

   public static string GenerateNonce(string extra = "")
   {
       string result = "";
       SHA1 sha1 = SHA1.Create();
   
       Random rand = new Random();
       StringBuilder sb = new StringBuilder(1024);
       while (result.Length < 32)
       {
           sb.Length = 0;
           string[] generatedRandoms = new string[4];
   
           for (int i = 0; i < 4; i++)
           {
               sb.Append(rand.Next());
           }
   
           sb.Append("|")
               .Append(extra);
   
           result += Convert.ToBase64String(
               sha1.ComputeHash(Encoding.ASCII.GetBytes(sb.ToString()))
               ).TrimEnd('=')
                .Replace("/", "")
                .Replace("+", "");
       }
   
       return result.Substring(0, 32);
   }